Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstallAPI] Simplify & improve symbol printing for diagnostics #85894

Merged
merged 1 commit into from
Mar 20, 2024
Merged

[InstallAPI] Simplify & improve symbol printing for diagnostics #85894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/include/clang/InstallAPI/DylibVerifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ class DylibVerifier {
/// Find matching dylib slice for target triple that is being parsed.
void assignSlice(const Target &T);

/// Gather annotations for symbol for error reporting.
std::string getAnnotatedName(const Record *R, SymbolContext &SymCtx,
bool ValidSourceLoc = true);

// Symbols in dylib.
llvm::MachO::Records Dylib;

Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/InstallAPI/MachO.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@
using SymbolFlags = llvm::MachO::SymbolFlags;
using RecordLinkage = llvm::MachO::RecordLinkage;
using Record = llvm::MachO::Record;
using EncodeKind = llvm::MachO::EncodeKind;
using GlobalRecord = llvm::MachO::GlobalRecord;
using ObjCContainerRecord = llvm::MachO::ObjCContainerRecord;
using ObjCInterfaceRecord = llvm::MachO::ObjCInterfaceRecord;
using ObjCCategoryRecord = llvm::MachO::ObjCCategoryRecord;
using ObjCIVarRecord = llvm::MachO::ObjCIVarRecord;
using ObjCIFSymbolKind = llvm::MachO::ObjCIFSymbolKind;
using Records = llvm::MachO::Records;
using RecordsSlice = llvm::MachO::RecordsSlice;
using BinaryAttrs = llvm::MachO::RecordsSlice::BinaryAttrs;
Expand Down
134 changes: 65 additions & 69 deletions clang/lib/InstallAPI/DylibVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ namespace installapi {

/// Metadata stored about a mapping of a declaration to a symbol.
struct DylibVerifier::SymbolContext {
// Name to use for printing in diagnostics.
std::string PrettyPrintName{""};

// Name to use for all querying and verification
// purposes.
std::string SymbolName{""};
Expand All @@ -30,11 +27,35 @@ struct DylibVerifier::SymbolContext {
bool Inlined = false;
};

static std::string
getAnnotatedName(const Record *R, EncodeKind Kind, StringRef Name,
bool ValidSourceLoc = true,
ObjCIFSymbolKind ObjCIF = ObjCIFSymbolKind::None) {
assert(!Name.empty() && "Need symbol name for printing");
static bool isCppMangled(StringRef Name) {
// InstallAPI currently only supports itanium manglings.
return (Name.starts_with("_Z") || Name.starts_with("__Z") ||
Name.starts_with("___Z"));
}

static std::string demangle(StringRef Name) {
// InstallAPI currently only supports itanium manglings.
if (!isCppMangled(Name))
return Name.str();
char *Result = llvm::itaniumDemangle(Name);
if (!Result)
return Name.str();

std::string Demangled(Result);
free(Result);
return Demangled;
}

std::string DylibVerifier::getAnnotatedName(const Record *R,
SymbolContext &SymCtx,
bool ValidSourceLoc) {
assert(!SymCtx.SymbolName.empty() && "Expected symbol name");

const StringRef SymbolName = SymCtx.SymbolName;
std::string PrettyName =
(Demangle && (SymCtx.Kind == EncodeKind::GlobalSymbol))
? demangle(SymbolName)
: SymbolName.str();

std::string Annotation;
if (R->isWeakDefined())
Expand All @@ -45,58 +66,47 @@ getAnnotatedName(const Record *R, EncodeKind Kind, StringRef Name,
Annotation += "(tlv) ";

// Check if symbol represents only part of a @interface declaration.
const bool IsAnnotatedObjCClass = ((ObjCIF != ObjCIFSymbolKind::None) &&
(ObjCIF <= ObjCIFSymbolKind::EHType));
const bool IsAnnotatedObjCClass =
((SymCtx.ObjCIFKind != ObjCIFSymbolKind::None) &&
(SymCtx.ObjCIFKind <= ObjCIFSymbolKind::EHType));

if (IsAnnotatedObjCClass) {
if (ObjCIF == ObjCIFSymbolKind::EHType)
if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::EHType)
Annotation += "Exception Type of ";
if (ObjCIF == ObjCIFSymbolKind::MetaClass)
if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::MetaClass)
Annotation += "Metaclass of ";
if (ObjCIF == ObjCIFSymbolKind::Class)
if (SymCtx.ObjCIFKind == ObjCIFSymbolKind::Class)
Annotation += "Class of ";
}

// Only print symbol type prefix or leading "_" if there is no source location
// tied to it. This can only ever happen when the location has to come from
// debug info.
if (ValidSourceLoc) {
if ((Kind == EncodeKind::GlobalSymbol) && Name.starts_with("_"))
return Annotation + Name.drop_front(1).str();
return Annotation + Name.str();
StringRef PrettyNameRef(PrettyName);
if ((SymCtx.Kind == EncodeKind::GlobalSymbol) &&
!isCppMangled(SymbolName) && PrettyNameRef.starts_with("_"))
return Annotation + PrettyNameRef.drop_front(1).str();
return Annotation + PrettyName;
}

if (IsAnnotatedObjCClass)
return Annotation + Name.str();
return Annotation + PrettyName;

switch (Kind) {
switch (SymCtx.Kind) {
case EncodeKind::GlobalSymbol:
return Annotation + Name.str();
return Annotation + PrettyName;
case EncodeKind::ObjectiveCInstanceVariable:
return Annotation + "(ObjC IVar) " + Name.str();
return Annotation + "(ObjC IVar) " + PrettyName;
case EncodeKind::ObjectiveCClass:
return Annotation + "(ObjC Class) " + Name.str();
return Annotation + "(ObjC Class) " + PrettyName;
case EncodeKind::ObjectiveCClassEHType:
return Annotation + "(ObjC Class EH) " + Name.str();
return Annotation + "(ObjC Class EH) " + PrettyName;
}

llvm_unreachable("unexpected case for EncodeKind");
}

static std::string demangle(StringRef Name) {
// InstallAPI currently only supports itanium manglings.
if (!(Name.starts_with("_Z") || Name.starts_with("__Z") ||
Name.starts_with("___Z")))
return Name.str();
char *Result = llvm::itaniumDemangle(Name);
if (!Result)
return Name.str();

std::string Demangled(Result);
free(Result);
return Demangled;
}

static DylibVerifier::Result updateResult(const DylibVerifier::Result Prev,
const DylibVerifier::Result Curr) {
if (Prev == Curr)
Expand Down Expand Up @@ -193,19 +203,18 @@ bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
// The decl represents a complete ObjCInterface, but the symbols in the
// dylib do not. Determine which symbol is missing. To keep older projects
// building, treat this as a warning.
if (!DR->isExportedSymbol(ObjCIFSymbolKind::Class))
if (!DR->isExportedSymbol(ObjCIFSymbolKind::Class)) {
SymCtx.ObjCIFKind = ObjCIFSymbolKind::Class;
PrintDiagnostic(DR->getLinkageForSymbol(ObjCIFSymbolKind::Class), R,
getAnnotatedName(R, SymCtx.Kind, SymCtx.PrettyPrintName,
/*ValidSourceLoc=*/true,
ObjCIFSymbolKind::Class),
getAnnotatedName(R, SymCtx),
/*PrintAsWarning=*/true);

if (!DR->isExportedSymbol(ObjCIFSymbolKind::MetaClass))
}
if (!DR->isExportedSymbol(ObjCIFSymbolKind::MetaClass)) {
SymCtx.ObjCIFKind = ObjCIFSymbolKind::MetaClass;
PrintDiagnostic(DR->getLinkageForSymbol(ObjCIFSymbolKind::MetaClass), R,
getAnnotatedName(R, SymCtx.Kind, SymCtx.PrettyPrintName,
/*ValidSourceLoc=*/true,
ObjCIFSymbolKind::MetaClass),
getAnnotatedName(R, SymCtx),
/*PrintAsWarning=*/true);
}
return true;
}

Expand All @@ -221,7 +230,7 @@ bool DylibVerifier::compareObjCInterfaceSymbols(const Record *R,
// At this point that means there was not a matching class symbol
// to represent the one discovered as a declaration.
PrintDiagnostic(DR->getLinkageForSymbol(SymCtx.ObjCIFKind), R,
SymCtx.PrettyPrintName);
SymCtx.SymbolName);
return false;
}

Expand All @@ -234,15 +243,15 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_library_missing_symbol)
<< SymCtx.PrettyPrintName;
<< getAnnotatedName(R, SymCtx);
});
return Result::Invalid;
}
if (DR->isInternal()) {
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_library_hidden_symbol)
<< SymCtx.PrettyPrintName;
<< getAnnotatedName(R, SymCtx);
});
return Result::Invalid;
}
Expand All @@ -269,7 +278,7 @@ DylibVerifier::Result DylibVerifier::compareVisibility(const Record *R,
}
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(), ID)
<< SymCtx.PrettyPrintName;
<< getAnnotatedName(R, SymCtx);
});
return Outcome;
}
Expand All @@ -293,14 +302,14 @@ DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::warn_header_availability_mismatch)
<< SymCtx.PrettyPrintName << IsDeclAvailable << IsDeclAvailable;
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
});
return Result::Ignore;
case VerificationMode::Pedantic:
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_header_availability_mismatch)
<< SymCtx.PrettyPrintName << IsDeclAvailable << IsDeclAvailable;
<< getAnnotatedName(R, SymCtx) << IsDeclAvailable << IsDeclAvailable;
});
return Result::Invalid;
case VerificationMode::ErrorsOnly:
Expand All @@ -313,23 +322,19 @@ DylibVerifier::Result DylibVerifier::compareAvailability(const Record *R,

bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
const Record *DR) {
std::string DisplayName =
Demangle ? demangle(DR->getName()) : DR->getName().str();

if (DR->isThreadLocalValue() && !R->isThreadLocalValue()) {
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_dylib_symbol_flags_mismatch)
<< getAnnotatedName(DR, SymCtx.Kind, DisplayName)
<< DR->isThreadLocalValue();
<< getAnnotatedName(DR, SymCtx) << DR->isThreadLocalValue();
});
return false;
}
if (!DR->isThreadLocalValue() && R->isThreadLocalValue()) {
Ctx.emitDiag([&]() {
SymCtx.FA->D->getLocation(),
Ctx.Diag->Report(diag::err_header_symbol_flags_mismatch)
<< SymCtx.PrettyPrintName << R->isThreadLocalValue();
<< getAnnotatedName(DR, SymCtx) << R->isThreadLocalValue();
});
return false;
}
Expand All @@ -338,16 +343,15 @@ bool DylibVerifier::compareSymbolFlags(const Record *R, SymbolContext &SymCtx,
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_dylib_symbol_flags_mismatch)
<< getAnnotatedName(DR, SymCtx.Kind, DisplayName)
<< R->isWeakDefined();
<< getAnnotatedName(DR, SymCtx) << R->isWeakDefined();
});
return false;
}
if (!DR->isWeakDefined() && R->isWeakDefined()) {
Ctx.emitDiag([&]() {
Ctx.Diag->Report(SymCtx.FA->D->getLocation(),
diag::err_header_symbol_flags_mismatch)
<< SymCtx.PrettyPrintName << R->isWeakDefined();
<< getAnnotatedName(R, SymCtx) << R->isWeakDefined();
});
return false;
}
Expand Down Expand Up @@ -457,10 +461,7 @@ DylibVerifier::Result DylibVerifier::verify(ObjCIVarRecord *R,

std::string FullName =
ObjCIVarRecord::createScopedName(SuperClass, R->getName());
SymbolContext SymCtx{
getAnnotatedName(R, EncodeKind::ObjectiveCInstanceVariable,
Demangle ? demangle(FullName) : FullName),
FullName, EncodeKind::ObjectiveCInstanceVariable, FA};
SymbolContext SymCtx{FullName, EncodeKind::ObjectiveCInstanceVariable, FA};
return verifyImpl(R, SymCtx);
}

Expand All @@ -485,11 +486,8 @@ DylibVerifier::Result DylibVerifier::verify(ObjCInterfaceRecord *R,
SymCtx.SymbolName = R->getName();
SymCtx.ObjCIFKind = assignObjCIFSymbolKind(R);

std::string DisplayName =
Demangle ? demangle(SymCtx.SymbolName) : SymCtx.SymbolName;
SymCtx.Kind = R->hasExceptionAttribute() ? EncodeKind::ObjectiveCClassEHType
: EncodeKind::ObjectiveCClass;
SymCtx.PrettyPrintName = getAnnotatedName(R, SymCtx.Kind, DisplayName);
SymCtx.FA = FA;

return verifyImpl(R, SymCtx);
Expand All @@ -504,8 +502,6 @@ DylibVerifier::Result DylibVerifier::verify(GlobalRecord *R,
SimpleSymbol Sym = parseSymbol(R->getName());
SymbolContext SymCtx;
SymCtx.SymbolName = Sym.Name;
SymCtx.PrettyPrintName =
getAnnotatedName(R, Sym.Kind, Demangle ? demangle(Sym.Name) : Sym.Name);
SymCtx.Kind = Sym.Kind;
SymCtx.FA = FA;
SymCtx.Inlined = R->isInlined();
Expand Down
4 changes: 2 additions & 2 deletions clang/test/InstallAPI/diagnostics-cpp.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
// RUN: --verify-mode=Pedantic -o %t/output.tbd --demangle 2> %t/errors.log
// RUN: FileCheck -input-file %t/errors.log %s

CHECK: warning: violations found for arm64-apple-macos13
CHECK: CPP.h:5:7: error: declaration has external linkage, but symbol has internal linkage in dynamic library 'vtable for Bar'
CHECK: warning: violations found for arm64-apple-macos13
CHECK: CPP.h:5:7: error: declaration has external linkage, but symbol has internal linkage in dynamic library 'vtable for Bar'
CHECK-NEXT: class Bar : Foo {
CHECK-NEXT: ^
CHECK-NEXT: CPP.h:5:7: error: declaration has external linkage, but symbol has internal linkage in dynamic library 'typeinfo for Bar'
Expand Down
Loading