Remove llvm change.

-Add lit test, although it uses a hard-coded path
-Make -external_symbolizer_path take precedence

In D94639#2496950, @dblaikie wrote:

In D94639#2496923, @akhuang wrote:

In D94639#2496892, @dblaikie wrote:

What parts of this are motivated by CodeView requirements (do functions have to have unique names in CV?)?

Might be worth rephrasing from "emit type information for function scopes" to "emit parent/context scopes for functions, using declarations for any scopes that are types". (my own fault, but reading this quickly, I assumed it was about emitting type information for the parameters (DWARF gmlt drops the parameters/types))

That sounds good

In D94639#2496892, @dblaikie wrote:

What parts of this are motivated by CodeView requirements (do functions have to have unique names in CV?)?
It'd be a bit unfortunate if we have divergence in -gmlt behavior between DWARF and CodeView due to different usage requirements, rather than format requirements - gmlt in DWARF I think uses only the unqualified name, though eventually combined with -fdebug-info-for-profiling which put the mangled name on such functions to make it clear for profilers. (though the unqualified name only behavior is still handy for the sanitizers - where the unqualified/ambiguous name is a "good enough" point for size V accuracy)

Remove other binary test

Add assembly test

Sure, I'm not really familiar with $ORIGIN - how does it work?

Update failing tests

Oh, I do need to fix the existing tests. Actually, the test I wrote probably belongs in that test directory.

sorry, I messed up the code when rebasing; should be fixed now

rebase

Add test case

In D92134#2417347, @rnk wrote:

Well, I *can* think of a way to test this from a .s file. Instead of using .cv_inline_line_table, you could use a sequence of .byte 0xNN, 0xNN directives. I think you can get the hex codes from llvm-pdbutil dump -symbols. How about it?

Add test; add warning in lld when it sees empty subsection

I haven't added any tests here; I could upload an msvc-built binary for testing this. Not sure if that's necessary?

avoid calling isa on a null value

Use WithColor::warning() to print the warning

fix typo

address comments

Thanks, looks like I'll try to see if libcxx code can be changed here.

thanks for writing tests for this! I think it looks good.

Ok, it seems like the general opinion here is that libc++ should be changed in some way and not ctor homing. I don't know who'd be responsible for changing the libc++ code, though.

more cleanup

Remove comment and print statement

address comment

+ @ldionne for libc++ input?

In D90719#2377485, @dblaikie wrote:

In D90719#2377324, @rnk wrote:

I had another thought, which is that even if it is UB, perhaps we really shouldn't be using UB as the basis for debug info emission. All programs have bugs, and most bugs invoke some form of UB. If we don't provide sufficient info when UB is involved, it can become harder to find the UB. The vtable type homing heuristic works because violating the heuristic assumptions typically results in a link error. Creating an object without calling the class's constructor is the kind of UB that is likely to manifest as runtime errors.

Which is to say, I'm in favor of Amy's change as written.

The same would be true for the ctor homing generally though, I think? If there was a user-defined ctor and someone chose not to call it, that'd be UB too. I'm not sure there's especially more cases of not calling implicit non-trivial ctors versus non-trivial explicit non-trivial ctors to diverge on this criteria?

I added an assembly test and removed the other test in llvm/test/tools, since it pretty much does the same thing.

Add an assembly test for symbolizer

No, chromium doesn't need this fixed in clang, but I didn't see a clear way to fix this in libc++. Do you think it should be fixed as a bug in libc++?

I think we want to emit floats as unsigned, so add a function to check if the
constant is a float.

rebase

Moved isUnsignedDIType to DebugHandlerBase to avoid copying it.

Reverted because my copied isUnsignedDIType function had problems (guess I copied it at first but then was messing with the code); I moved it to DebugHandlerBase.h so it doesn't need to be copied.

Patch to emit signed ints for other S_CONSTANTs: https://reviews.llvm.org/D90199

Fix test failures; remove assert for signed APSInts.

Whoops, forgot to check the tests. Apparently mapEncodedInteger asserts if a signed APInt is not less than 0. I just removed the assert because it doesn't seem necessary?