On Tue, 6 Aug 2024 18:58:42 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> > No, it's not when building libjvm.a. It's when linking the final elf > > executable using libjvm.a (or libnet.a, etc) that's created with ld -r or > > objcopy. During the final elf executable linking time, user could include > > additional desired linker flags, such as -Wl,--icf=safe. That's the case > > where things can run into trouble. > > Ok, I see. The static library that we generate is not usable by a later > invocation of lld. Yes, that's the problem. > > Just out of curiousity, is running objcopy required to reproduce this > problem, or is it enough to run lld first with -r and then for linking the > static library into an executable? If it is the latter, I think it is a > really sorry implementation in lld, if they can't even use their own output. > (If objcopy messes up the file, then it's a different story -- I can't really > blame them if that would happen). Either `objcopy` **or** `ld -r` could invalidate the `.llvm_addrsig` according to the linked blog (which has the nice description of the related details). While searching for additional information today to help make things more clear for our discussion in this thread, I just found `strip` could also invalidate the section. https://sourceware.org/bugzilla/show_bug.cgi?id=23817 mentions about the issue with `strip`. Also, according to the comments in https://sourceware.org/bugzilla/show_bug.cgi?id=23817, it seems tools might not be updated to address the corrupted `.llvm_addrsig` section. > > > We don't have control of the linker flags that users might want to use. > > Well, that is indeed correct, but we can tell them that there is a know > problem with lld that it cannot link these binaries with --icf=safe. (At this > point I am primarily treating this as an lld bug/limitation.) > > > > If you were to remove these sections, then the linker will believe that > > > all functions are safe to merge. > > > How did you arrive the conclusion? > > I quoted the part from one of the links you sent: "Functions that are not > mentioned in the section are not address-taken (no one takes its pointer), so > they are safe to merge." https://llvm.org/docs/Extensions.html#sht-llvm-addrsig-section-address-significance-table probably (found today) is more clear: "Any sections referred to by symbols that are not marked as address-significant in any object file may be safely merged ..." So if symbols are treated as "address-significant", then referring object file may not be safely merged. See more related comments below. > > Prior to reading this PR, I did not know about icf, so I have no other > knowledge about how it works. I interpreted the paragraph above as it would > be risky to remove this section (given that it might have contained relevant > content before it was mangled by ld -r/objcopy), since it would apply that > there were no functions that needed preserving. > > > When .llvm_addrsig section is removed (not present), my understanding is > > that the linker would only do conservative identical code folding safely. > > Well, my contrary question is then: How did you arrive at this conclusion? > I'd like to get some definite, authoritative documentation on how icf works > in the presence of a mangled .llvm_addrsig section vs an intentionally > removed .llvm_addrsig section. My understanding is based on the descriptions in the linked blog, specifically the part quoted from the blog in my pervious comment (quoted below again to be more clear): "ld.lld --icf=safe uses a special section .llvm_addrsig (LLVM address significance table, type SHT_LLVM_ADDRSIG) produced by Clang -faddrsig....If the section is absent, ld.lld is conservative and assumes every section defining a symbol in the table is address significant." > > As I see it, unless there is documentation stating that removing the > .llvm_addrsig section is safe to combine with --icf=safe, then the only > reliable option is to not use icf at all. Reasonably, lld should fall back to > that behavior if it is missing .llvm_addrsig sections, but apparently it > chooses to fail instead. > > I'm not sure what this "conservative" icf you talk about means. My > understanding, after trying to read the docs now, is that there is only three > modes that ICF works in: safe, all or none. If this understanding is correct, > then you only have two options if lld can't do safe icf: namely all or none. > And all seems to be a dangerous choice, since it can lead to UB. > > > Removing .llvm_addrsig is safe. > > You keep saying this. Please show me some documentation that supports this > claim. Unfortunately, the blog article was the most comprehensive summary about the issue with the corrupted .llvm_addrsig that I was able to find. I could not find any official documentation about removing the `.llvm_addrsig` section. In this case, I trust the expert. Additionally, the comment in https://lists.llvm.org/pipermail/llvm-dev/2018-May/123528.html (I found today) matches with the blog info, specifically: "but now I see you assume all symbols are address-significant if there's no address-significance table in the file" Anyway, we can't **safely** keep the corrupted `.llvm_addrsig` section in `libjvm.a` and etc. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20265#issuecomment-2272346203
