- User Since
- Aug 17 2017, 6:34 AM (87 w, 6 d)
Mon, Apr 22
Sun, Apr 21
Mon, Apr 15
Existing tools have learned various heuristics to avoid 0 DW_AT_low_pc collision, but I am not sure what they'd do if those relocations resolve to -1. llvm-symbolizer is certainly good (I have checked a few >examples - I'm learning it recently), but what about other tools?
@ruiu What do you think about using UINT64_MAX-1 as address to which relocations for deleted sections would be resolved ? Does it make sense ?
Fri, Apr 12
@MaskRay Hi, Since you changed status of review into "Needs Revision" - What changes do you think should be done ?
Wed, Apr 10
Tue, Apr 9
__libc_csu_init is in libc.a(elf-init.o) and has no debug info. It should symbolize to ?? but not_used.cpp is incorrectly returned. ?? -> not_used.cpp
I'm not sure if making zero DW_AT_low_pc special for executable/DSO in DWARFDebugAranges is a good idea.
I think the current handling in lld is quite good, no need to special case on if (!Sym.getOutputSection()). The same problem can be reproduced for ld.bfd and gold.
I thought that address 0 is a good candidate to represent an invalid address, as nullptr is usually represented by address 0. I wonder why we want to use UINT64_MAX-2 instead of 0.
Mon, Apr 8
Wed, Apr 3
@rocallahan Would you mind to share the performance results of that patch, please ? Similar to above table for rusoto test, but with timings ...
Tue, Apr 2
Fri, Mar 29
added indentation and corrected sections string.
Thu, Mar 28
addressed all comments, except indentation one, which I did not understand yet.
addressed all comments.
I think at this point the "debug info cabal" is more in favor of fragmenting the DWARF emitted by the compiler, so that it can be gc'd in tandem with the related functions.
Mar 25 2019
abandon revision because of already integrated duplicated revision https://reviews.llvm.org/D59490
Mar 23 2019
It seems you indented the description by 2. That may be why the revision is not automatically closed..
Mar 22 2019
Please move this down at least far enough that it doesn't involve separately
computing the file name and opening the file. Where it's a "well, if you didn't
specify a section index, we'll use the first section that covers this address".
updated tests as requested.
What does GDB make of -1/-2???
updated tests and addressed comments.
Mar 21 2019
addressed comments. added tests for 32 bit.
Does this do the right thing for a 32-bit target? 64-bit is common, but not universal.
Mar 20 2019
This seems like it might be worth a broader discussion about these sort of cases - probably llvm-dev,
maybe with some DWARF folks (though the usual LLVM debug info cabal (myself, @aprantl, @probinson,
@JDevlieghere, and @echristo) is probably sufficient to get a rough idea of what might be a good general approach).
Is this logically correct? I wonder what is the point of emitting a relocation section pointing to a nonexistent section.
- used UINT_MAX-1 as value for relocations referenced deleted section
- changed title of review
- updated tests
- I tried to use UINT64_MAX-1 as a address value for lost debug info :
Mar 19 2019
What do other linkers (gold and gnu binutils, for instance) do in this case?
I am not sure about using grep and sed in the test. I see it used in other parts of llvm but not inside lld.
Thanks for fixing this! Please check comments.
Mar 16 2019
Since that is compile time issue - not neccessary to delay it for long time. LGTM.
Mar 15 2019
David, there is a connected review - https://reviews.llvm.org/D59189 It cures compilation error after D58194 Could you take a look on it please ?
I think that change is OK and it is good to do in spite of decision made in this review.
Mar 14 2019
In case Undef specified then there could be several sections with matching address ranges.
It seems to me that DWARFDebugLineTable improper place to make such a decision - which of them should be taken as a result.
This in fact high level decision, not low level.
I moved handling Undef case from Symbolizer into SymbolizableObjectFile. Please check whether it is proper place.
Mar 13 2019
This looks OK for me(taking into account the comment). Let final approve would be done by David, since He reviewed original patch.
David, Is it OK for you ?
addressed all comments.
Mar 12 2019
David, Could you also review https://reviews.llvm.org/D58959 please ? It seems not depending on decision discussed here. So it looks like it could already be integrated.
Mar 11 2019
There is a duplicated review https://reviews.llvm.org/D59189 , I`ve already commented there ...
Oh I missed perf jit case, excuse me...
Mar 10 2019
Mar 9 2019
Mar 8 2019
Mar 5 2019
added check for section_end()
I separated ExecutionEngine into another review - https://reviews.llvm.org/D58959
Mar 4 2019
Mar 2 2019
David, Thank you for the comments! I skipped code style issues for the moment. please check the comments for allowing setting SectionIndex into Undef state and for the necessity of getModuleSectionIndexForAddress() routine. After we will decide on these questions I will handle code style issues.
Mar 1 2019
Feb 28 2019
Feb 27 2019
Feb 26 2019
addressed latest comments.
Feb 24 2019
Feb 19 2019
George, Thank you for the comments. I should not rely on context and needed to put detailed description from the scratch. I changed test description and used your compacted asm now.
Feb 18 2019
deleted LLD test as it was requested. Modified llvm-objdump test to not to use clang.
Feb 15 2019
George, I added test for LLD. But.... It already pass for LLD without this fix. The reason for that is following quickfix :