- User Since
- Jan 18 2017, 2:49 AM (126 w, 6 d)
FWIW, I think this test is generally testing the behaviour of libObject code, so arguably belongs exactly where it is.
Add extra notes to explain demangling differences a bit.
Make two-letter options use double-dashes, make the --all description less fragile to new options, and standardise on "display" instead of "print" for consistency throughout the document.
Thanks @MaskRay. I'm going to land this shortly, to reduce the number of in-flight patches I have on the docs, but I'll happily take further feedback for improvements if anybody has any.
Hi @ychen, would you mind posting a patch to update the docs for this behaviour change? I didn't mention it previously, because rL364019, which added --disassemble-functions to the listed options, was still under review, but it landed before this change did.
There seems to be a general consensus to change these docs to RST (see also the discussion in D63292 and on the mailing list). As such, I'm abandoning this revision. I'll make changes in the coming days to switch things over.
Thanks all. I'm going to land this shortly. I'm happy to make further changes if there's any remaining issues.
Fri, Jun 21
I do not think it is a very often case to have duplicated symbol/sections name, so I selected the less intrusive way.
LGTM, with one comment fix.
Removed references to llvm-dwarfdump from test/Support/check-default-options.txt.
I got the man pages building from the rst using ninja docs-llvm-man, and a set of man pages were built, including one for llvm-objdump that mirrors the html doc. I don't see any reason why this can't be used: it's built by the sphinx build bot (see http://lab.llvm.org:8011/builders/llvm-sphinx-docs/builds/32333 for an example build).
Do you know how good rst is to generate the man page? If it is less ideal (mdoc is a complex format...), personally I think it is fine to stick with the current manually maintained manpage...
LGTM, with the suggested comment changes, but please wait for the other patch, and @MaskRay's agreement.
@MaskRay, @peter.smith, is it just the PLT that is specially handled? Perhaps we could just special-case that in llvm-objdump? I think there's clear precedence for this for other versions of objdump, so I'm okay with that difference in behaviour.
Hmmm... I'm not sure we need any special behaviour at all in the name field of the syntax, and the suggested syntax is not obvious to me. Surely if yaml2obj sees two symbols with the same name, it should just generate two such symbols?
May I ask you to start a thread on https://lists.llvm.org/pipermail/llvm-dev/2019-June to discuss this?
LGTM. Maybe give it a couple of days before committing for a chance for others to say something.
Thu, Jun 20
D63565 needs to land first for the llvm-dwarfdump test to work, due to an odd difference in behaviour between -h and --help in llvm-dwarfdump.
@majnemer, I've added you as it looks like you've worked on the COFF llvm-nm side a fair bit. How do the COFF aspects look?
Reword undefined and weak symbol code text. Also update 'n' text following behaviour change.
I've added a couple of other reviewers who have made recent changes to the Path files.
I think this looks pretty good, but I'd like D63583 to land first if possible.
LGTM too. I'll update the doc patch.
So I'm happy with this approach, but I'd like somebody else to confirm that they are too.
LGTM, with the suggested change, but please wait for the other reviewers to confirm too.
Okay, LGTM, but @MaskRay probably should confirm he's happy enough for now (we can improve the STT_SECTION bit later, if we want).
Wed, Jun 19
Update text of 's' to specify which sections it applies to more precisely.
Apologies for the delay in coming back to this. It's been a busy few days.
You're going to need to account for program headers in your layout code. If you take a look at llvm-objcopy, a lot of the layout work is done based on those, rather than the sections, because sections within segments should move with their segment.
LGTM, with one comment fix.
Can you persuade git/svn to do a rename of the test rather than a delete and add? That would make the diff easier to see.
Hi @mmpozulp, just wondering if you are going to come back to this some time soon, or if you need any further assistance with this issue?
Tue, Jun 18