-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[llvm-symbolizer] Recognize AIX big archive #150401
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.
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
d98ed02
64639e1
f5a357e
305ef99
66d5d11
27b4f10
647f98e
629e7a5
8c71818
f0be9a8
bfa845e
9c14b38
f8aeb3e
0952fcf
6be8d1a
12dd6e5
68b886f
d0f0a1d
fe5a9c2
e2083eb
12bf16a
0595460
4d46767
315b98c
b37409b
30e4387
08f38ae
ca85990
844906c
74d4e23
183197a
c68b4d9
7407769
4da5ebd
b0f0705
86f2023
f034d90
c6d8b90
e4d6fed
f57d66a
c3acb24
136bde7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
#include "llvm/DebugInfo/PDB/PDBContext.h" | ||
#include "llvm/DebugInfo/Symbolize/SymbolizableObjectFile.h" | ||
#include "llvm/Demangle/Demangle.h" | ||
#include "llvm/Object/Archive.h" | ||
#include "llvm/Object/BuildID.h" | ||
#include "llvm/Object/COFF.h" | ||
#include "llvm/Object/ELFObjectFile.h" | ||
|
@@ -33,7 +34,6 @@ | |
#include "llvm/Support/FileSystem.h" | ||
#include "llvm/Support/MemoryBuffer.h" | ||
#include "llvm/Support/Path.h" | ||
#include "llvm/Object/Archive.h" | ||
#include <cassert> | ||
#include <cstring> | ||
|
||
|
@@ -323,7 +323,7 @@ bool checkFileCRC(StringRef Path, uint32_t CRCHash) { | |
|
||
bool getGNUDebuglinkContents(const ObjectFile *Obj, std::string &DebugName, | ||
uint32_t &CRCHash) { | ||
if (!Obj || !isa<ObjectFile>(Obj)) | ||
if (!Obj) | ||
return false; | ||
for (const SectionRef &Section : Obj->sections()) { | ||
StringRef Name; | ||
|
@@ -564,26 +564,23 @@ LLVMSymbolizer::getOrCreateObjectPair(const std::string &Path, | |
std::string DbgObjPath = DbgObj->getFileName().str(); | ||
auto BinIter = BinaryForPath.find(DbgObjPath); | ||
if (BinIter != BinaryForPath.end()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could I get an explanation for why this behaviour has changed, please. Also, I assume you have test coverage for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not answered. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some iterations, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by "in some iterations"? Is this actually related to your addition of big archive support? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As in the other comments, please explain in detail from the entry point what the code path is that could hit here without the binary being cached, please, because I don't see it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a plain object file (t.o) the Updating Hope this helps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. It's going to take me some time to go over this and determine if there's a better approach. I suspect there is, but I'm not sure yet what that is. |
||
BinIter->second.pushEvictor([this, I = Pair.first]() { | ||
ObjectPairForPathArch.erase(I); | ||
}); | ||
BinIter->second.pushEvictor( | ||
[this, I = Pair.first]() { ObjectPairForPathArch.erase(I); }); | ||
} | ||
return Res; | ||
} | ||
|
||
Expected<ObjectFile *> LLVMSymbolizer::getOrCreateObjectFromArchive(StringRef ArchivePath, | ||
StringRef MemberName, | ||
const std::string &ArchName) { | ||
Expected<ObjectFile *> LLVMSymbolizer::getOrCreateObjectFromArchive( | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
StringRef ArchivePath, StringRef MemberName, StringRef ArchName) { | ||
Binary *Bin = nullptr; | ||
auto Pair = BinaryForPath.emplace(ArchivePath.str(), OwningBinary<Binary>()); | ||
if (!Pair.second) { | ||
Bin = Pair.first->second->getBinary(); | ||
recordAccess(Pair.first->second); | ||
} else { | ||
Expected<OwningBinary<Binary>> ArchiveOrErr = createBinary(ArchivePath); | ||
if (!ArchiveOrErr) { | ||
if (!ArchiveOrErr) | ||
return ArchiveOrErr.takeError(); | ||
} | ||
|
||
CachedBinary &CachedBin = Pair.first->second; | ||
CachedBin = std::move(ArchiveOrErr.get()); | ||
|
@@ -593,43 +590,46 @@ Expected<ObjectFile *> LLVMSymbolizer::getOrCreateObjectFromArchive(StringRef Ar | |
Bin = CachedBin->getBinary(); | ||
} | ||
|
||
if (!Bin || !isa<object::Archive>(Bin)) | ||
object::Archive *Archive = dyn_cast_if_present<object::Archive>(Bin); | ||
if (!Archive) | ||
return errorCodeToError(object_error::invalid_file_type); | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
object::Archive *Archive = cast<object::Archive>(Bin); | ||
Error Err = Error::success(); | ||
|
||
// On AIX, archives can contain multiple members with same name but different types | ||
// We need to check all matches and find one that matches both name and architecture | ||
|
||
// On AIX, archives can contain multiple members with same name but different | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
// types. We need to check all matches and find one that matches both name and | ||
// architecture. | ||
for (auto &Child : Archive->children(Err, /*SkipInternal=*/true)) { | ||
Expected<StringRef> NameOrErr = Child.getName(); | ||
if (!NameOrErr) | ||
continue; | ||
if (*NameOrErr == llvm::sys::path::filename(MemberName)) { | ||
Expected<std::unique_ptr<object::Binary>> MemberOrErr = Child.getAsBinary(); | ||
continue; | ||
if (*NameOrErr == sys::path::filename(MemberName)) { | ||
|
||
Expected<std::unique_ptr<object::Binary>> MemberOrErr = | ||
Child.getAsBinary(); | ||
if (!MemberOrErr) | ||
continue; | ||
continue; | ||
|
||
std::unique_ptr<object::Binary> Binary = std::move(*MemberOrErr); | ||
if (auto *Obj = dyn_cast<object::ObjectFile>(Binary.get())) { | ||
#if defined(_AIX) | ||
Triple::ArchType ObjArch = Obj->makeTriple().getArch(); | ||
Triple RequestedTriple; | ||
RequestedTriple.setArch(Triple::getArchTypeForLLVMName(ArchName)); | ||
if (ObjArch != RequestedTriple.getArch()) | ||
continue; | ||
#endif | ||
ArchiveCacheKey CacheKey{ArchivePath.str(), MemberName.str(), ArchName}; | ||
ArchiveCacheKey CacheKey{ArchivePath.str(), MemberName.str(), | ||
ArchName.str()}; | ||
auto I = ObjectForArchivePathAndArch.find(CacheKey); | ||
if (I != ObjectForArchivePathAndArch.end()) | ||
return I->second.get(); | ||
|
||
auto CachedObj = std::unique_ptr<ObjectFile>(Obj); | ||
auto NewEntry = ObjectForArchivePathAndArch.emplace( | ||
CacheKey, std::move(CachedObj)); | ||
auto NewEntry = | ||
ObjectForArchivePathAndArch.emplace(CacheKey, std::move(CachedObj)); | ||
Binary.release(); | ||
BinaryForPath.find(ArchivePath.str())->second.pushEvictor( | ||
[this, Iter = NewEntry.first]() { ObjectForArchivePathAndArch.erase(Iter); }); | ||
BinaryForPath.find(ArchivePath.str()) | ||
->second.pushEvictor([this, Iter = NewEntry.first]() { | ||
ObjectForArchivePathAndArch.erase(Iter); | ||
}); | ||
return NewEntry.first->second.get(); | ||
} | ||
} | ||
|
@@ -642,13 +642,15 @@ Expected<ObjectFile *> LLVMSymbolizer::getOrCreateObjectFromArchive(StringRef Ar | |
Expected<ObjectFile *> | ||
LLVMSymbolizer::getOrCreateObject(const std::string &Path, | ||
const std::string &ArchName) { | ||
// First check for archive(member) format - more efficient to check closing paren first | ||
// First check for archive(member) format - more efficient to check closing | ||
// paren first. | ||
size_t CloseParen = Path.rfind(')'); | ||
if (CloseParen != std::string::npos && CloseParen == Path.length() - 1) { | ||
size_t OpenParen = Path.rfind('(', CloseParen); | ||
if (OpenParen != std::string::npos) { | ||
StringRef ArchivePath = StringRef(Path).substr(0, OpenParen); | ||
StringRef MemberName = StringRef(Path).substr(OpenParen + 1, CloseParen - OpenParen - 1); | ||
StringRef MemberName = | ||
StringRef(Path).substr(OpenParen + 1, CloseParen - OpenParen - 1); | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
return getOrCreateObjectFromArchive(ArchivePath, MemberName, ArchName); | ||
} | ||
} | ||
|
@@ -803,10 +805,8 @@ LLVMSymbolizer::getOrCreateModuleInfo(StringRef ModuleName) { | |
if (ModuleOrErr) { | ||
auto I = Modules.find(ModuleName); | ||
auto BinIter = BinaryForPath.find(BinaryName); | ||
if (BinIter != BinaryForPath.end()) | ||
BinIter->second.pushEvictor([this, I]() { | ||
Modules.erase(I); | ||
}); | ||
if (BinIter != BinaryForPath.end()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment: what's the purpose of the logic change and what test coverage is there for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some iterations the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, what is the new code path that causes the crash that you've discussed? I've looked at the code and I can't see it: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a plain object file (t.o) the Updating Hope this helps |
||
BinIter->second.pushEvictor([this, I]() { Modules.erase(I); }); | ||
} | ||
return ModuleOrErr; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
// Test archive member recognition by name (ELF format) | ||
|
||
// UNSUPPORTED: target={{.*}}-aix{{.*}} | ||
// REQUIRES: system-linux || system-aix | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
// Generate object files from YAML | ||
// RUN: yaml2obj -o %t-1.o %S/Inputs/big-archive-elf-1.yaml | ||
|
@@ -9,14 +9,13 @@ | |
// RUN: rm -f %t.a | ||
// RUN: llvm-ar crv %t.a %t-1.o %t-2.o | ||
|
||
// Verify archive contents | ||
// RUN: llvm-ar tv %t.a | FileCheck %s --check-prefix=CHECK-ARCHIVE | ||
// CHECK-ARCHIVE: {{.*}}-1.o | ||
// CHECK-ARCHIVE: {{.*}}-2.o | ||
|
||
// Test symbolization by member name (using just base names) | ||
midhuncodes7 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
// RUN: llvm-symbolizer --default-arch=ppc64 --obj="%t.a(%t-1.o)" 0x0 | FileCheck %s --check-prefix=CHECK-1 | ||
// RUN: llvm-symbolizer --default-arch=ppc64 --obj="%t.a(%t-2.o)" 0x0 | FileCheck %s --check-prefix=CHECK-2 | ||
// RUN: MEMBER1=$(basename %t-1.o) | ||
// RUN: llvm-symbolizer --default-arch=ppc64le --obj="%t.a($MEMBER1)" 0x0 | FileCheck %s --check-prefix=CHECK-1 | ||
// RUN: llvm-symbolizer --obj="%t.a($MEMBER1):ppc64le" 0x0 | FileCheck %s --check-prefix=CHECK-1 | ||
// RUN: MEMBER2=$(basename %t-2.o) | ||
// RUN: llvm-symbolizer --default-arch=ppc64le --obj="%t.a($MEMBER2)" 0x0 | FileCheck %s --check-prefix=CHECK-2 | ||
// RUN: llvm-symbolizer --obj="%t.a($MEMBER2):ppc64le" 0x0 | FileCheck %s --check-prefix=CHECK-2 | ||
// CHECK-1: foo1 | ||
// CHECK-2: foo2 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,31 @@ | ||
// Test big archive recognition and error handling in llvm-symbolizer | ||
// REQUIRES: system-aix, target={{.*}}-aix{{.*}} | ||
// REQUIRES: system-linux || system-aix | ||
jh7370 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
// Generate object files | ||
// RUN: yaml2obj -o %t-32.o %S/Inputs/big-archive-32.yaml | ||
// RUN: yaml2obj -o %t-64.o %S/Inputs/big-archive-64.yaml | ||
|
||
// Create archive with same-named members using different modes | ||
// RUN: rm -f %t.a | ||
// RUN: cp %t-32.o %t.o && llvm-ar -X32 crv %t.a %t.o | ||
// RUN: cp %t-64.o %t.o && llvm-ar -X64 qv %t.a %t.o | ||
|
||
// RUN: cp %t-32.o %t.o | ||
// RUN: llvm-ar %if system-aix %{-X32%} crv %t.a %t.o | ||
// RUN: cp %t-64.o %t.o | ||
// RUN: llvm-ar %if system-aix %{-X64 rv%} %else %{qv%} %t.a %t.o | ||
|
||
// Verify archive contains two members with same name | ||
// RUN: llvm-ar tv -X32_64 %t.a | FileCheck %s --check-prefix=CHECK-ARCHIVE | ||
// RUN: llvm-ar tv %if system-aix %{-X32_64%} %t.a | FileCheck %s --check-prefix=CHECK-ARCHIVE | ||
// CHECK-ARCHIVE: {{.*}}symbolize-big-archive-xcoff.test.tmp.o{{$}} | ||
// CHECK-ARCHIVE: {{.*}}symbolize-big-archive-xcoff.test.tmp.o{{$}} | ||
|
||
// Test successful symbolization | ||
// RUN: llvm-symbolizer --default-arch=ppc --obj="%t.a(%t.o)" 0x0 | FileCheck %s --check-prefix=CHECK-32 | ||
// RUN: llvm-symbolizer --default-arch=ppc64 --obj="%t.a(%t.o)" 0x0 | FileCheck %s --check-prefix=CHECK-64 | ||
// RUN: MEMBER=$(basename %t.o) | ||
// RUN: llvm-symbolizer --default-arch=ppc --obj="%t.a($MEMBER)" 0x0 | FileCheck %s --check-prefix=CHECK-32 | ||
// RUN: llvm-symbolizer --default-arch=ppc64 --obj="%t.a($MEMBER)" 0x0 | FileCheck %s --check-prefix=CHECK-64 | ||
// RUN: llvm-symbolizer --obj="%t.a($MEMBER):ppc" 0x0 | FileCheck %s --check-prefix=CHECK-32 | ||
// RUN: llvm-symbolizer --obj="%t.a($MEMBER):ppc64" 0x0 | FileCheck %s --check-prefix=CHECK-64 | ||
// CHECK-32: foo | ||
// CHECK-64: foo | ||
|
||
// Test error cases | ||
// RUN: not llvm-symbolizer --obj="%t.a(nonexistent.o)" 0x1000 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR | ||
|
||
// CHECK-ERROR: error: {{.*}}No object file for requested architecture |
Uh oh!
There was an error while loading. Please reload this page.