probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (244 w, 6 d)

Recent Activity

Yesterday

probinson added a comment to D42082: Add DWARF for discriminated unions.

I'd argue against a new tag, because it raises the barrier to debugger support for Rust.

In practice the barrier is the same. Neither gdb nor lldb read variant parts, either. I'm writing the gdb support as part of my Rust work there, and elsewhere I'm also working on an lldb plugin to support Rust.

Maybe there are proprietary debuggers or something, but ...

Wed, Jan 17, 2:40 PM · debug-info
probinson added a comment to D42082: Add DWARF for discriminated unions.

I think this approach actually makes more sense, because surely the discriminant is something whose placement cannot vary.

Wed, Jan 17, 1:07 PM · debug-info
probinson added a comment to D41761: Introduce llvm.nospeculateload intrinsic.

It would be awesome to have static analysis rules to help identify *where* to put these intrinsics. Is somebody working on that? Or did I miss it?

Wed, Jan 17, 8:41 AM
probinson added a comment to D42082: Add DWARF for discriminated unions.

The DWARF spec says: "If the variant part has a discriminant, the discriminant is represented by a separate debugging information entry which is a child of the variant part entry. This entry has the form of a structure data member entry. The variant part entry will have a DW_AT_descr attribute whose value is a reference to the member entry for the discriminant."

Wed, Jan 17, 8:37 AM · debug-info

Tue, Jan 16

probinson added a comment to D42082: Add DWARF for discriminated unions.

As I poke around the web looking for how a Rust enum-with-value works, it appears that Rust has what in Pascal would be a tagged variant record with no invariant part and no default variant. It's the web, of course, so it might be lying to me, but that's what I see, and it's about 90% of what you need for Pascal. The DWARF spec does not provide an example of a variant record, but I would expect to see something along these lines in the final DWARF:

DW_TAG_struct_type // type entry for the enum-with-fields
  DW_TAG_variant_part // because DWARF says variant_part is owned by the struct_type
      DW_AT_discr // reference to the discriminator field
    DW_TAG_member // this is the discriminator field, where the enum value is stored
    DW_TAG_variant // wrapper for the first variant
        DW_AT_discr_value // first discriminant value
      DW_TAG_member ... // member(s) for the first variant
    DW_TAG_variant // wrapper for the second variant
        DW_AT_discr_value // second discriminant value 
      DW_TAG_member ... // member(s) for the second variant
    // etc

Is that what you are trying to produce?

Tue, Jan 16, 2:50 PM · debug-info
probinson added a comment to D42082: Add DWARF for discriminated unions.

DWARF discriminated unions were likely designed to support Pascal variant records, which do allow multiple fields per variant. If we're going to support discriminated unions (which seems reasonable) then it ought to be general enough to handle the Pascal case.

Tue, Jan 16, 11:00 AM · debug-info

Fri, Jan 12

probinson committed rL322430: XFAIL a test on Darwin, line-table stuck on DWARF 2.
XFAIL a test on Darwin, line-table stuck on DWARF 2
Fri, Jan 12, 5:41 PM
probinson committed rC322413: [DWARFv5] Have -gdwarf-5 generate MD5 checksums.
[DWARFv5] Have -gdwarf-5 generate MD5 checksums
Fri, Jan 12, 2:22 PM
probinson committed rL322413: [DWARFv5] Have -gdwarf-5 generate MD5 checksums.
[DWARFv5] Have -gdwarf-5 generate MD5 checksums
Fri, Jan 12, 2:22 PM
probinson closed D42011: [DWARFv5] Enable MD5 checksums.
Fri, Jan 12, 2:22 PM · debug-info
probinson closed D42011: [DWARFv5] Enable MD5 checksums.
Fri, Jan 12, 2:22 PM · debug-info
probinson created D42011: [DWARFv5] Enable MD5 checksums.
Fri, Jan 12, 2:12 PM · debug-info
probinson committed rL322400: Try to fix more bots after r322391.
Try to fix more bots after r322391
Fri, Jan 12, 12:56 PM
probinson committed rL322394: Add toothpicks to test from r322391.
Add toothpicks to test from r322391
Fri, Jan 12, 11:59 AM
probinson committed rL322391: [DWARFv5] CodeGen support for MD5 file checksums.
[DWARFv5] CodeGen support for MD5 file checksums
Fri, Jan 12, 11:19 AM
probinson closed D41926: [DWARFv5] CodeGen support for MD5 checksum.
Fri, Jan 12, 11:19 AM · debug-info
probinson added a comment to D41957: Utility for checking out llvm, clang, and associated tools and configuring a build folder.

There's a medium-term goal of migrating to git, so an svn-based script seems like the wrong direction.
I've been using the experimental git mono-repo for a while and I'm finding it pretty convenient. A sample configuration script for setting up a useful subset of projects and canonical CMake parameters seems like a good helpful starting point.

Fri, Jan 12, 11:08 AM
probinson added a comment to D41985: Rewrite debugger tuning test case to not depend on apple sections.

Agreed.

Fri, Jan 12, 10:29 AM

Thu, Jan 11

probinson added a comment to D41926: [DWARFv5] CodeGen support for MD5 checksum.

Thanks! I'll commit this tomorrow.

Thu, Jan 11, 5:36 PM · debug-info
probinson updated the diff for D41926: [DWARFv5] CodeGen support for MD5 checksum.

Simplify a bit, making assumptions the verifier will check for us.

Thu, Jan 11, 4:32 PM · debug-info
probinson committed rL322314: Tighten up DIFile verifier for checksums.
Tighten up DIFile verifier for checksums
Thu, Jan 11, 2:05 PM
probinson closed D41965: Tighten up DIFile verifier checks on checksum.
Thu, Jan 11, 2:05 PM · debug-info
probinson added a comment to D41956: [docs] Tweak update to Phabricator docs about setting repository for diffs uploaded via web.

So, I just posted a review without specifying the Repository for the diff, but I did put it on the review, and it auto-subscribed llvm-commits.
I suspect there's no real reason to put it on a diff, and we shouldn't tell people to do that.

Thu, Jan 11, 1:59 PM
probinson added a comment to D41926: [DWARFv5] CodeGen support for MD5 checksum.

See D41965 for verifier change.

Thu, Jan 11, 1:55 PM · debug-info
probinson created D41965: Tighten up DIFile verifier checks on checksum.
Thu, Jan 11, 1:53 PM · debug-info
probinson added a comment to D41956: [docs] Tweak update to Phabricator docs about setting repository for diffs uploaded via web.

And there are no subscribers on this review...

Thu, Jan 11, 1:15 PM
probinson added a comment to D41956: [docs] Tweak update to Phabricator docs about setting repository for diffs uploaded via web.

Thinking about the Desired Visible Effect, the auto-subscription would be on the Revision and not on the Diff, assuming that it's the subscriber list on the Revision that determines where the emails go.
But that would be sensible from the end-user's point of view, which doesn't mean it's actually how the software works. :-)

Thu, Jan 11, 11:14 AM
probinson added a comment to D41926: [DWARFv5] CodeGen support for MD5 checksum.

So, the checksum will come in from the front-end as a string in DIFile. It wants to be binary in the .o file. What happens in-between is maybe not optimal, but has to address a couple of considerations.
First, where is it stored and who owns the memory? As a StringRef it's not clear, but as an MD5Result it really ought to be owned by MCContext (the CodeView path does the same thing). AsmPrinter does this allocation and conversion.
Second, from AsmPrinter it goes to "the streamer" which might be emitting an object file or might be emitting a text file, but AsmPrinter (despite its name) doesn't know which. For type and memory safety I chose to have the MC API take an MD5Result* instead of yet another StringRef. For emitting a text file, MCAsmStreamer converts it back to text, and then the AsmParser reads the text and converts it back to binary before passing it to "the (object-file) streamer." AsmParser needs to validate the string when it does this conversion. Then the object-file streamer has binary data and just emits it to the object file without any further fuss. For emitting an object-file directly, AsmPrinter passes it along as binary data and there are no additional conversions in the path.

Thu, Jan 11, 11:09 AM · debug-info

Wed, Jan 10

probinson added inline comments to D41926: [DWARFv5] CodeGen support for MD5 checksum.
Wed, Jan 10, 5:33 PM · debug-info
probinson added a comment to D41926: [DWARFv5] CodeGen support for MD5 checksum.

This was not immediately obvious to me: Why can we now have empty DIFiles?
LGTM otherwise.

Wed, Jan 10, 5:30 PM · debug-info
probinson added a comment to D41924: dagcombine: Transfer debug information when folding (zext (truncate x)) -> (zext (truncate x)).

I'm scared to actually approve it, but I can see the clear parallel with the AND case just afterwards, so it seems like the right thing to do.

Wed, Jan 10, 5:24 PM
probinson added a comment to D41924: dagcombine: Transfer debug information when folding (zext (truncate x)) -> (zext (truncate x)).
the dagcombine rule (zext (truncate x)) -> (zext (truncate x)).
Wed, Jan 10, 5:00 PM
probinson created D41926: [DWARFv5] CodeGen support for MD5 checksum.
Wed, Jan 10, 4:49 PM · debug-info

Tue, Jan 9

probinson closed D41846: [DWARFv5] MC support for MD5 checksum.

Committed in rL322134. (Forgot to put the Differential Revision tag in the commit message.)

Tue, Jan 9, 3:56 PM · debug-info
probinson committed rL322134: [DWARFv5] MC support for MD5 file checksums.
[DWARFv5] MC support for MD5 file checksums
Tue, Jan 9, 3:33 PM
probinson updated the diff for D41846: [DWARFv5] MC support for MD5 checksum.

Address review feedback.

Tue, Jan 9, 2:42 PM · debug-info
probinson added inline comments to D41846: [DWARFv5] MC support for MD5 checksum.
Tue, Jan 9, 9:30 AM · debug-info

Mon, Jan 8

probinson created D41846: [DWARFv5] MC support for MD5 checksum.
Mon, Jan 8, 5:19 PM · debug-info
probinson accepted D41757: Add a config note and fix a config variable regarding CCACHE support..

LGTM

Mon, Jan 8, 1:08 PM
probinson added a comment to D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre...

But retpoline doesn't make every indirection more expensive any more or less than zapping the predictor... You only build the code running in the privileged domain with retpoline, not all of the code, and they both accomplish very similar things.

Mon, Jan 8, 12:00 PM

Fri, Jan 5

probinson added a comment to D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre...

For AMD processors we may be able to handle indirect jumps via a simpler lfence mechanism. Indirect calls may still require retpoline. If this turns out to be the right solution for AMD processors we may need to put some code in to support this.

Yeah, if it ends up that we want non-retpoline mitigations for AMD we can and should add them. One hope I have is that this patch is at least generically *sufficient* (when paired with correct RSB filling) even if it suboptimal in some cases and we end up adding more precise tools later.

Fri, Jan 5, 7:21 PM
probinson added a comment to D41757: Add a config note and fix a config variable regarding CCACHE support..

See inline comment. I think that makes it clear enough where to look for details about what values to use for the MAXSIZE/DIR parameters.

Fri, Jan 5, 3:25 PM
probinson added a comment to D41757: Add a config note and fix a config variable regarding CCACHE support..

I know off-list I had suggested not bothering with detailed descriptions of LLVM_CCACHE_SIZE and LLVM_CCACHE_DIR, but now I am not so sure. If those variables map directly to ccache command-line arguments, we should say that here, and people can figure out how to set them by looking at the appropriate help. If the variables are more complicated, we should add separate entries for them in this doc.

Fri, Jan 5, 11:23 AM

Thu, Jan 4

probinson updated subscribers of D41697: [DebugInfo][Metadata] Add support for a DIExpression as 'count' field of DISubrange..

My motivation for disallowing constants after introducing DIExpressions is that I want to avoid having more than one way to represent the same thing, since it complicates the code and brings potential for bugs.

Thu, Jan 4, 11:44 AM

Wed, Jan 3

probinson added a comment to D41695: [Metadata] Extend 'count' field of DISubrange to take a metadata node.
In D41695#967020, @rnk wrote:

I'm concerned this will blow up memory usage of commonly encountered array types.

Wed, Jan 3, 4:35 PM
probinson added a comment to D41695: [Metadata] Extend 'count' field of DISubrange to take a metadata node.

Should the lower bound also become an expression? My FORTRAN days are long gone but my impression is that the lower bound isn't necessarily fixed at compile-time, in more modern editions of the language.

Wed, Jan 3, 2:33 PM
probinson committed rL321757: Calculate size of buffer instead of using a magic value..
Calculate size of buffer instead of using a magic value.
Wed, Jan 3, 12:31 PM
probinson committed rC321757: Calculate size of buffer instead of using a magic value..
Calculate size of buffer instead of using a magic value.
Wed, Jan 3, 12:31 PM
probinson closed D41421: Eliminate a magic number. NFC..
Wed, Jan 3, 12:30 PM

Thu, Dec 28

probinson added a comment to D41615: [DebugInfo] Don't crash when given invalid DWARFv5 line table prologue..

In a real object file it should never be zero. There are a bunch of places in the unittests where I construct one with version and addrsize both zero; those would have to be fixed if we wanted to have a check somewhere other than for a form that actually cares about that stuff.

Thu, Dec 28, 10:32 AM · debug-info

Tue, Dec 26

probinson committed rC321457: Fix comment typo in r321312..
Fix comment typo in r321312.
Tue, Dec 26, 10:03 AM
probinson committed rL321457: Fix comment typo in r321312..
Fix comment typo in r321312.
Tue, Dec 26, 10:02 AM
probinson added a comment to D40987: Rewrite the cached map used for locating the most precise DIE among inlined subroutines for a given address..

Sorry, missed this somehow... I didn't really look at the second layer, but I have a suggestion for the initial tree walk.

Tue, Dec 26, 9:06 AM

Thu, Dec 21

probinson committed rL321323: Update test after r321312.
Update test after r321312
Thu, Dec 21, 3:20 PM
probinson committed rCTE321323: Update test after r321312.
Update test after r321312
Thu, Dec 21, 3:20 PM
probinson committed rL321312: [AST] Incorrectly qualified unscoped enumeration as template actual parameter..
[AST] Incorrectly qualified unscoped enumeration as template actual parameter.
Thu, Dec 21, 1:48 PM
probinson committed rC321312: [AST] Incorrectly qualified unscoped enumeration as template actual parameter..
[AST] Incorrectly qualified unscoped enumeration as template actual parameter.
Thu, Dec 21, 1:48 PM
probinson closed D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter..
Thu, Dec 21, 1:48 PM · debug-info

Wed, Dec 20

probinson added inline comments to D41146: [DWARF] DWARF v5: Rework of string offsets table reader.
Wed, Dec 20, 3:59 PM
probinson added a comment to D40849: CodeGen: support an extension to pass linker options on ELF.
  • I know the PlayStation proprietary linker has support for a similar feature (as James has been discussing) and IIRC there were private patches in the compiler for it. Is it in scope for this change to allow deleting those private patches from the PlayStation compiler?
Wed, Dec 20, 3:43 PM
probinson added a project to D41466: [Unroll][DebugInfo] Propagate loop body's debug location to epilog preheader: debug-info.
Wed, Dec 20, 3:04 PM · debug-info
probinson accepted D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter..

With the GDB test results and LLDB able to handle it, this LGTM.
Carlos, do you have commit access?

Wed, Dec 20, 12:51 PM · debug-info
probinson added a comment to D41264: Fix faulty assertion for void type in debug info.
0x00000068:     DW_TAG_typedef
                  DW_AT_name	("_Nodeptr")
                  DW_AT_decl_file	("/tmp/t.cpp")
                  DW_AT_decl_line	(2)

0x0000006f:     NULL

this also looks weird, but is apparently sort of legal DWARF:

For example, in C and C++ the language implementation can provide an unspecified type entry with the name “void” which can be referenced by the type attribute of pointer types and typedef declarations for ’void’.

although it would be nicer to use DW_TAG_unspecified_type instead.

Based on this it looks like we should allow this case.

Wed, Dec 20, 8:59 AM · debug-info

Dec 18 2017

probinson committed rL321011: Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header.".
Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header."
Dec 18 2017, 11:09 AM

Dec 15 2017

probinson committed rL320888: Revert "Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header."".
Revert "Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header.""
Dec 15 2017, 3:22 PM
probinson committed rL320886: Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header.".
Recommit "[DWARFv5] Dump an MD5 checksum in the line-table header."
Dec 15 2017, 2:58 PM
probinson committed rL320857: Revert "[DWARFv5] Dump an MD5 checksum in the line-table header.".
Revert "[DWARFv5] Dump an MD5 checksum in the line-table header."
Dec 15 2017, 12:30 PM
probinson added inline comments to D41090: [DWARFv5] Dump MD5 checksum from line-table header.
Dec 15 2017, 11:53 AM · debug-info
probinson committed rL320852: [DWARFv5] Dump an MD5 checksum in the line-table header..
[DWARFv5] Dump an MD5 checksum in the line-table header.
Dec 15 2017, 11:53 AM
probinson closed D41090: [DWARFv5] Dump MD5 checksum from line-table header.
Dec 15 2017, 11:53 AM · debug-info
probinson added a comment to D41090: [DWARFv5] Dump MD5 checksum from line-table header.

Ping.

Dec 15 2017, 9:50 AM · debug-info

Dec 14 2017

probinson committed rL320727: [MC] Allow .file directives to be out-of-order.
[MC] Allow .file directives to be out-of-order
Dec 14 2017, 10:47 AM

Dec 13 2017

probinson added a comment to D39622: Fix type name generation in DWARF for template instantiations with enum types and template specializations.

Philosophically, mangled names and DWARF information serve different purposes, and I don't think you will find one true solution where both of them can yield the same name that everyone will be happy with. Mangled names exist to provide unique and reproducible identifiers for the "same" entity across compilation units. They are carefully specified (for example) to allow a linker to associate a reference in one object file to a definition in a different object file, and be guaranteed that the association is correct. A demangled name is a necessarily context-free translation of the mangled name into something that has a closer relationship to how a human would think of or write the name of the thing, but isn't necessarily the only way to write the name of the thing.

Dec 13 2017, 3:07 PM · debug-info, Restricted Project
probinson added a project to D39622: Fix type name generation in DWARF for template instantiations with enum types and template specializations: debug-info.
Dec 13 2017, 2:37 PM · debug-info, Restricted Project

Dec 11 2017

probinson accepted D41084: [dsymutil] Accept DWARF4 line tables..

Awesome. LGTM.

Dec 11 2017, 2:58 PM · debug-info
probinson added a comment to D41044: Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code.

I should note we've had at least one request to make this specifiable per-function, which would mean defining an attribute to control the emission of the fake-use intrinsics. Doing the feature that way would be consistent with 'optnone'.

Dec 11 2017, 2:16 PM · debug-info
probinson created D41090: [DWARFv5] Dump MD5 checksum from line-table header.
Dec 11 2017, 1:13 PM · debug-info
probinson requested changes to D41084: [dsymutil] Accept DWARF4 line tables..
Dec 11 2017, 12:44 PM · debug-info
probinson added inline comments to D41084: [dsymutil] Accept DWARF4 line tables..
Dec 11 2017, 12:43 PM · debug-info

Dec 8 2017

probinson committed rL320216: Fix out-of-order stepping behavior in programs with sunk instructions..
Fix out-of-order stepping behavior in programs with sunk instructions.
Dec 8 2017, 4:17 PM
probinson closed D39933: Summary: Fix out-of-order stepping behavior in programs with sunk instructions by committing rL320216: Fix out-of-order stepping behavior in programs with sunk instructions..
Dec 8 2017, 4:17 PM
probinson added a comment to D40712: [Driver] Add flag enabling the function stack size section that was added in r319430.

The title says "Add cc1 option..." which to me implies a "cc1 only" option. What you're actually doing is adding a driver option. Please update the title.

Dec 8 2017, 3:48 PM
probinson accepted D39933: Summary: Fix out-of-order stepping behavior in programs with sunk instructions.

LGTM again.

Dec 8 2017, 2:07 PM
probinson added inline comments to D40948: Switch Clang's default C++ language target to C++14.
Dec 8 2017, 12:23 PM

Dec 7 2017

probinson accepted D39950: [DebugInfo] Stable sort symbols to remove non-deterministic ordering.

LGTM then, so @mgrang can proceed with the (IMO rather valuable) iteration order work. And redoing aranges can go on "the list."

Dec 7 2017, 6:35 PM · debug-info
probinson added a reviewer for D40948: Switch Clang's default C++ language target to C++14: rjmccall.

+rjmccall for the codegen bits.

Dec 7 2017, 11:44 AM
probinson added a comment to D39950: [DebugInfo] Stable sort symbols to remove non-deterministic ordering.

Well... I'm not fully convinced that the stable_sort isn't just papering over some other problem, but it seems to be producing the correct final result. If @echristo is okay with it, I'm okay with it.

Dec 7 2017, 11:00 AM · debug-info

Dec 6 2017

probinson added a comment to D38002: Re-submit r289925 (Update .debug_line section version to match DWARF version).

Ah, thanks for the help Davide! Looks like you can do this:
if (context.getObjectFileInfo()->getTargetTriple().isOSDarwin()) LineTableVersion = 2;
and that should take care of it.

Dec 6 2017, 2:42 PM · debug-info
probinson added a comment to D38002: Re-submit r289925 (Update .debug_line section version to match DWARF version).

Let me see if I can find a way to force version 2 for Darwin only. I really don't want to revert again, it took months to get Chrome to fix their tools and reverting would block further work.

Dec 6 2017, 2:33 PM · debug-info
probinson added a comment to D38002: Re-submit r289925 (Update .debug_line section version to match DWARF version).

More generally, DWARF changes should not impact the lldb test suite (apparently one of the bots, green dragon, failed, and now has been red for days).

Dec 6 2017, 2:23 PM · debug-info

Dec 5 2017

probinson committed rL319827: [DWARFv5] Emit v5 line table header..
[DWARFv5] Emit v5 line table header.
Dec 5 2017, 12:35 PM
probinson closed D40741: [DWARFv5] Emit v5 line-table header by committing rL319827: [DWARFv5] Emit v5 line table header..
Dec 5 2017, 12:35 PM · debug-info

Dec 4 2017

probinson added a comment to D40512: [Debugify] Add a pass to test debug info preservation.

Thanks this is a great idea!

I assume this will only ever be used via opt, so you could move the pass to llvm/tools/opt to keep the size of the llvm libraries smaller.

Dec 4 2017, 5:50 PM · debug-info
probinson added inline comments to D40777: [dsymutil] Add -verify option to run DWARF verifier after linking..
Dec 4 2017, 5:38 PM
probinson committed rL319699: Re-submit r289925 (Update .debug_line section version to match DWARF version).
Re-submit r289925 (Update .debug_line section version to match DWARF version)
Dec 4 2017, 1:28 PM
probinson closed D38002: Re-submit r289925 (Update .debug_line section version to match DWARF version) by committing rL319699: Re-submit r289925 (Update .debug_line section version to match DWARF version).
Dec 4 2017, 1:28 PM · debug-info

Dec 1 2017

probinson added a comment to D40741: [DWARFv5] Emit v5 line-table header.

Yeah, I copy-pasted the old code and then hacked on it for the v5 format. I should have looked at it all as new code.

Dec 1 2017, 4:27 PM · debug-info
probinson updated the diff for D40741: [DWARFv5] Emit v5 line-table header.

Range-base for loops; tidy up commentary.

Dec 1 2017, 4:26 PM · debug-info
probinson added a comment to D39950: [DebugInfo] Stable sort symbols to remove non-deterministic ordering.

@echristo does it make sense to emit aranges without the symbols being emitted yet?

Dec 1 2017, 4:00 PM · debug-info
probinson created D40741: [DWARFv5] Emit v5 line-table header.
Dec 1 2017, 11:12 AM · debug-info