[llvm] r243095 - [dsymutil] Make the triple detection more strict.
Frédéric Riss
friss at apple.com
Fri Jul 24 10:41:55 PDT 2015
> On Jul 24, 2015, at 10:23 AM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> On Fri, Jul 24, 2015 at 9:35 AM Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> On Jul 24, 2015, at 9:26 AM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>>
>> The mach-o ness of this particular code is a little depressing. Any reason why?
>
> Because of the universal binaries. As we have fat files with multiple variants of
> ARM, or x86_64, it’s important to get exact triples in the mach-o case. I still check
> that arch in other cases, but it’s just a Triple.getArch() check that isn’t sufficient
> to differentiate the variants.
> Note that the mach-o specific getTriple() helper is used only in the mach-o specific
> debug map parser.
>
>
> Guess I'll need to look through the code more. Something called "BinaryHolder" didn't scream "MachO specific thing". One more thing to add to the list.
Unfortunately, you’ll see that the BinaryHolder has grown quite a lot of mach-o specific code. It’s still a generic ObjectFile access device, but to be able to transparently handle the universal binary, every method of it needs to be mach-o universal aware.
In the patch that actually implements that (r243096) the high-level interface of the BinaryHolder gets that treatment:
- const object::ObjectFile &Get();
+ ErrorOr<const object::ObjectFile &> Get(const Triple &T);
- template <typename ObjectFileType> const ObjectFileType &GetAs();
+ ErrorOr<const ObjectFileType &> GetAs(const Triple &T);
Which doesn’t leak the macho-ness of the implementation.
(And if it’s just for the getTriple() helper, I can make it more general to accept any ObjectFile, I just don’t have a user for that)
Fred
> -eric
>
>> -eric
>>
>> On Thu, Jul 23, 2015 at 11:46 PM Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> Author: friss
>> Date: Fri Jul 24 01:41:04 2015
>> New Revision: 243095
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=243095&view=rev <http://llvm.org/viewvc/llvm-project?rev=243095&view=rev>
>> Log:
>> [dsymutil] Make the triple detection more strict.
>>
>> MachOObjectFile offers a method for detecting the correct triple, use
>> it instead of the previous approximation. This doesn't matter right
>> now, but it will become important for mach-o universal (fat) binaries.
>>
>> Modified:
>> llvm/trunk/test/tools/dsymutil/X86/frame-1.test
>> llvm/trunk/test/tools/dsymutil/X86/frame-2.test
>> llvm/trunk/test/tools/dsymutil/X86/odr-1.test
>> llvm/trunk/test/tools/dsymutil/archive-timestamp.test
>> llvm/trunk/test/tools/dsymutil/debug-map-parsing.test
>> llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test
>> llvm/trunk/tools/dsymutil/BinaryHolder.cpp
>> llvm/trunk/tools/dsymutil/BinaryHolder.h
>> llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp
>>
>> Modified: llvm/trunk/test/tools/dsymutil/X86/frame-1.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/frame-1.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/frame-1.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/X86/frame-1.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/X86/frame-1.test Fri Jul 24 01:41:04 2015
>> @@ -9,7 +9,7 @@
>> # link twice the same file using this made-up debug map:
>>
>> ---
>> -triple: 'i386-unknown-unknown-macho'
>> +triple: 'i386-apple-darwin'
>> objects:
>> - filename: frame-dw2.o
>> symbols:
>>
>> Modified: llvm/trunk/test/tools/dsymutil/X86/frame-2.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/frame-2.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/frame-2.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/X86/frame-2.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/X86/frame-2.test Fri Jul 24 01:41:04 2015
>> @@ -12,7 +12,7 @@
>> # appears again. This is a behavior we inherited from dsymutil-classic
>> # but this should be fixed (see comment in patchFrameInfoForObject())
>> ---
>> -triple: 'i386-unknown-unknown-macho'
>> +triple: 'i386-apple-darwin'
>> objects:
>> - filename: frame-dw2.o
>> symbols:
>>
>> Modified: llvm/trunk/test/tools/dsymutil/X86/odr-1.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/odr-1.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/odr-1.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/X86/odr-1.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/X86/odr-1.test Fri Jul 24 01:41:04 2015
>> @@ -14,7 +14,7 @@
>> # Totally made up debug map to test ODR uniquing
>>
>> ---
>> -triple: 'x86_64-unknown-unknown-macho'
>> +triple: 'x86_64-apple-darwin'
>> objects:
>> - filename: odr1.o
>> symbols:
>>
>> Modified: llvm/trunk/test/tools/dsymutil/archive-timestamp.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/archive-timestamp.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/archive-timestamp.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/archive-timestamp.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/archive-timestamp.test Fri Jul 24 01:41:04 2015
>> @@ -6,7 +6,7 @@
>> # CHECK: warning: {{.*}}libbasic.a(basic3.macho.x86_64.o): {{[Nn]o}} such file
>>
>> ---
>> -triple: 'x86_64-unknown-unknown-macho'
>> +triple: 'x86_64-apple-darwin'
>> objects:
>> - filename: '/Inputs/libbasic.a(basic2.macho.x86_64.o)'
>> timestamp: 141869239
>>
>> Modified: llvm/trunk/test/tools/dsymutil/debug-map-parsing.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/debug-map-parsing.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/debug-map-parsing.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/debug-map-parsing.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/debug-map-parsing.test Fri Jul 24 01:41:04 2015
>> @@ -9,7 +9,7 @@ Check that We can parse the debug map of
>>
>> CHECK-NOT: error
>> CHECK: ---
>> -CHECK: triple: 'x86_64-unknown-unknown-macho'
>> +CHECK: triple: 'x86_64-apple-darwin'
>> CHECK: filename:{{.*}}/Inputs/basic1.macho.x86_64.o
>> CHECK-DAG: sym: _main, objAddr: 0x0000000000000000, binAddr: 0x0000000100000EA0, size: 0x00000024
>> CHECK: filename{{.*}}/Inputs/basic2.macho.x86_64.o
>> @@ -28,7 +28,7 @@ Check that we can parse the debug-map of
>>
>> CHECK-LTO-NOT: error
>> CHECK-LTO: ---
>> -CHECK-LTO: triple: 'x86_64-unknown-unknown-macho'
>> +CHECK-LTO: triple: 'x86_64-apple-darwin'
>> CHECK-LTO: /Inputs/basic-lto.macho.x86_64.o
>> CHECK-LTO-DAG: sym: _bar, objAddr: 0x0000000000000050, binAddr: 0x0000000100000F90, size: 0x00000024
>> CHECK-LTO-DAG: sym: _baz, objAddr: 0x0000000000000658, binAddr: 0x0000000100001000, size: 0x00000000
>> @@ -51,7 +51,7 @@ CHECK-ARCHIVE-NEXT: found member in cur
>> CHECK-ARCHIVE-NEXT: trying to open {{.*}}/libbasic.a(basic3.macho.x86_64.o)'
>> CHECK-ARCHIVE-NEXT: found member in current archive.
>> CHECK-ARCHIVE: ---
>> -CHECK-ARCHIVE: triple: 'x86_64-unknown-unknown-macho'
>> +CHECK-ARCHIVE: triple: 'x86_64-apple-darwin'
>> CHECK-ARCHIVE: /Inputs/basic1.macho.x86_64.o
>> CHECK-ARCHIVE-DAG: sym: _main, objAddr: 0x0000000000000000, binAddr: 0x0000000100000EA0, size: 0x00000024
>> CHECK-ARCHIVE: /Inputs/./libbasic.a(basic2.macho.x86_64.o)
>> @@ -72,7 +72,7 @@ NOT-FOUND: cannot open{{.*}}"/Inputs/bas
>> NOT-FOUND: cannot open{{.*}}"/Inputs/basic2.macho.x86_64.o": {{[Nn]o}} such file
>> NOT-FOUND: cannot open{{.*}}"/Inputs/basic3.macho.x86_64.o": {{[Nn]o}} such file
>> NOT-FOUND: ---
>> -NOT-FOUND-NEXT: triple: 'x86_64-unknown-unknown-macho'
>> +NOT-FOUND-NEXT: triple: 'x86_64-apple-darwin'
>> NOT-FOUND-NEXT: ...
>>
>> Check that we correctly error out on invalid executatble.
>>
>> Modified: llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test (original)
>> +++ llvm/trunk/test/tools/dsymutil/yaml-object-address-rewrite.test Fri Jul 24 01:41:04 2015
>> @@ -5,7 +5,7 @@
>> # rewrite these addresses to the right values.
>> #
>> # CHECK: ---
>> -# CHECK-NEXT: triple:{{.*}}'x86_64-unknown-unknown-macho'
>> +# CHECK-NEXT: triple:{{.*}}'x86_64-apple-darwin'
>> # CHECK-NEXT: objects:
>> # CHECK-NEXT: filename:{{.*}}/Inputs/basic1.macho.x86_64.o
>> # CHECK-NEXT: timestamp: 0
>> @@ -28,7 +28,7 @@
>> # CHECK-NOT: { sym:
>> # CHECK-NEXT: ...
>> ---
>> -triple: 'x86_64-unknown-unknown-macho'
>> +triple: 'x86_64-apple-darwin'
>> objects:
>> - filename: /Inputs/basic1.macho.x86_64.o
>> symbols:
>>
>> Modified: llvm/trunk/tools/dsymutil/BinaryHolder.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.cpp?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/tools/dsymutil/BinaryHolder.cpp (original)
>> +++ llvm/trunk/tools/dsymutil/BinaryHolder.cpp Fri Jul 24 01:41:04 2015
>> @@ -13,11 +13,21 @@
>> //===----------------------------------------------------------------------===//
>>
>> #include "BinaryHolder.h"
>> +#include "llvm/Object/MachO.h"
>> #include "llvm/Support/raw_ostream.h"
>>
>> namespace llvm {
>> namespace dsymutil {
>>
>> +Triple BinaryHolder::getTriple(const object::MachOObjectFile &Obj) {
>> + // If a ThumbTriple is returned, use it instead of the standard
>> + // one. This is because the thumb triple always allows to create a
>> + // target, whereas the non-thumb one might not.
>> + Triple ThumbTriple;
>> + Triple T = Obj.getArch(nullptr, &ThumbTriple);
>> + return ThumbTriple.getArch() ? ThumbTriple : T;
>> +}
>> +
>> void BinaryHolder::changeBackingMemoryBuffer(
>> std::unique_ptr<MemoryBuffer> &&Buf) {
>> CurrentArchive.reset();
>>
>> Modified: llvm/trunk/tools/dsymutil/BinaryHolder.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.h?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/BinaryHolder.h?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/tools/dsymutil/BinaryHolder.h (original)
>> +++ llvm/trunk/tools/dsymutil/BinaryHolder.h Fri Jul 24 01:41:04 2015
>> @@ -14,6 +14,7 @@
>> #ifndef LLVM_TOOLS_DSYMUTIL_BINARYHOLDER_H
>> #define LLVM_TOOLS_DSYMUTIL_BINARYHOLDER_H
>>
>> +#include "llvm/ADT/Triple.h"
>> #include "llvm/Object/Archive.h"
>> #include "llvm/Object/Error.h"
>> #include "llvm/Object/ObjectFile.h"
>> @@ -108,6 +109,8 @@ public:
>> template <typename ObjectFileType> const ObjectFileType &GetAs() {
>> return cast<ObjectFileType>(*CurrentObjectFile);
>> }
>> +
>> + static Triple getTriple(const object::MachOObjectFile &Obj);
>> };
>> }
>> }
>>
>> Modified: llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp?rev=243095&r1=243094&r2=243095&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp?rev=243095&r1=243094&r2=243095&view=diff>
>> ==============================================================================
>> --- llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp (original)
>> +++ llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp Fri Jul 24 01:41:04 2015
>> @@ -103,13 +103,6 @@ void MachODebugMapParser::switchToNewDeb
>> CurrentDebugMapObject = &Result->addDebugMapObject(Path, Timestamp);
>> }
>>
>> -static Triple getTriple(const object::MachOObjectFile &Obj) {
>> - Triple TheTriple("unknown-unknown-unknown");
>> - TheTriple.setArch(Triple::ArchType(Obj.getArch()));
>> - TheTriple.setObjectFormat(Triple::MachO);
>> - return TheTriple;
>> -}
>> -
>> /// This main parsing routine tries to open the main binary and if
>> /// successful iterates over the STAB entries. The real parsing is
>> /// done in handleStabSymbolTableEntry.
>> @@ -120,7 +113,7 @@ ErrorOr<std::unique_ptr<DebugMap>> MachO
>>
>> const MachOObjectFile &MainBinary = *MainBinOrError;
>> loadMainBinarySymbols();
>> - Result = make_unique<DebugMap>(getTriple(MainBinary));
>> + Result = make_unique<DebugMap>(BinaryHolder::getTriple(MainBinary));
>> MainBinaryStrings = MainBinary.getStringTableData();
>> for (const SymbolRef &Symbol : MainBinary.symbols()) {
>> const DataRefImpl &DRI = Symbol.getRawDataRefImpl();
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150724/4720ea2c/attachment.html>
More information about the llvm-commits
mailing list