[PATCH] D58234: [llvm-objcopy] Add --add-symbol
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 18 07:01:37 PST 2019
jhenderson added inline comments.
================
Comment at: tools/llvm-objcopy/CopyConfig.h:66
+ StringRef SectionName;
+ unsigned long long Value;
+ uint8_t Type;
----------------
jhenderson wrote:
> evgeny777 wrote:
> > rupprecht wrote:
> > > uint64_t
> > Like I said in D58173 this will not work, because `uint64_t` is not `unsigned long long` on 64 bit platforms
> I don't understand what you mean by this? Symbol values are by definition in the ELF spec up to 64-bit in size, so `uint64_t` (which is 64-bits in size) is the correct type. `unsigned long long` is only guaranteed to be at least as big as `unsigned long` which in turn is only guaranteed to be as big as `unisgned int` etc.
>
> Related note: The bitness of the platform does not define the size of types by itself. For example, on Windows, `unsigned long` is always 32 bits, both for 32-bit Windows and 64-bit Windows. On the other hand, `unsigned long long` is 64 bits on both.
Okay, I now see what the problem is. It's to do with the call to `getAsUnsignedInteger`, later on right?
I think this issue could be solved by defining an unsigned long version of `getAsUnsignedInteger`. That way, it should work regardless of the underlying type of `uint64_t`. Implementing that would be my preference to working around this by using primitive types in llvm-objcopy rather than fixed-width types.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58234/new/
https://reviews.llvm.org/D58234
More information about the llvm-commits
mailing list