debug-infoProject
ActivePublic

Recent Activity

Today

vleschuk added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

@jhenderson Please take a look when you have time.

Sun, Aug 19, 3:47 AM · debug-info
vleschuk updated the diff for D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..
  • Changed error codes to more appropriate ones
  • Fixed formatting
Sun, Aug 19, 3:46 AM · debug-info
vleschuk added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

I just noticed that llvmObject has an object_error equivalent to std::errc, with a make_error_code method allowing us to turn them into std::error_code for use with the new function. In particular, there are unexpected_eof and parser_failed errors, which might be more appropriate for certain situations in this file. What do you think?

Sun, Aug 19, 3:32 AM · debug-info
vleschuk added inline comments to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..
Sun, Aug 19, 3:26 AM · debug-info

Thu, Aug 16

vleschuk added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

Hi @vleschuk, I just wanted to quickly check-in on this to see if you need any help with making further improvements as suggested?

Thu, Aug 16, 11:38 PM · debug-info

Wed, Aug 15

bjope created D50788: [LiveDebugVariables] Avoid faulty addDefsFromCopies in computeIntervals.
Wed, Aug 15, 10:08 AM · debug-info
jhenderson added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

Hi @vleschuk, I just wanted to quickly check-in on this to see if you need any help with making further improvements as suggested?

Wed, Aug 15, 4:45 AM · debug-info

Tue, Aug 14

vsk added a dependent revision for D49887: [DebugInfo] Add support for DWARF5 call site-related attributes: D50478: Add support for artificial tail call frames.
Tue, Aug 14, 12:25 PM · debug-info
calixte added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.

DILocation is a subclass of MDNode which is itself a subclass of Metadata. And in Metadata there is "unsigned char Storage" which uses only 2 bits so 6 bits are available here.
Maybe I could add "unsigned char ImplicitCode : 1" and "unsigned char Storage: 7" in class Metadata and so don't increase the DILocation size.
What do you think ?

Tue, Aug 14, 4:42 AM · debug-info

Mon, Aug 13

friss added a comment to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.

The dsymutil part LGTM

Mon, Aug 13, 2:37 PM · debug-info
aprantl added inline comments to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
Mon, Aug 13, 10:18 AM · debug-info
aprantl added inline comments to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
Mon, Aug 13, 10:17 AM · debug-info
vsk added inline comments to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
Mon, Aug 13, 10:08 AM · debug-info
aprantl added inline comments to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
Mon, Aug 13, 9:25 AM · debug-info

Fri, Aug 10

vsk added inline comments to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
Fri, Aug 10, 4:00 PM · debug-info
vsk updated the diff for D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.

Thanks for your feedback @dblaikie! I've addressed some of it and responded to the rest inline.

Fri, Aug 10, 4:00 PM · debug-info
dblaikie added a comment to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.

probably leave this a bit more to @aprantl and @JDevlieghere (figure they can speak more to the LLDB needs/tradeoffs/etc that this is targeting) - but there's a first pass.

Fri, Aug 10, 12:31 PM · debug-info
vsk updated subscribers of D48994: Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass.

@thegameg and @kparzysz, would either of you have some time to take a look?

Fri, Aug 10, 9:59 AM · debug-info
yurydelendik added a comment to D48994: Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass.

@MatzeB or @vsk, can you recommend a reviewer for this patch?

Fri, Aug 10, 8:05 AM · debug-info

Thu, Aug 9

vsk planned changes to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.

Dsymutil will need to learn to resolve addresses in DW_AT_call_return_pc. Currently, I think it's skipping over them, which means that lldb can't use them.

Thu, Aug 9, 6:54 PM · debug-info
vsk updated the diff for D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.
  • Add verifier & bitcode tests, rebase.
  • Add complete DWARF generation support for call site entries (i.e, one call site entry per call, DW_AT_call_tail_call emission, and DW_AT_call_return_pc emission).
Thu, Aug 9, 2:46 PM · debug-info

Wed, Aug 8

JDevlieghere closed D49381: [DWARF] Unclamp line table version on Darwin for v5 and later..
Wed, Aug 8, 2:17 PM · debug-info
aprantl accepted D49381: [DWARF] Unclamp line table version on Darwin for v5 and later..

Looks right.

Wed, Aug 8, 8:44 AM · debug-info
JDevlieghere updated the diff for D49381: [DWARF] Unclamp line table version on Darwin for v5 and later..

Add comment to tests explaining why this works on Darwin.

Wed, Aug 8, 5:38 AM · debug-info

Tue, Aug 7

probinson abandoned D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.

I'm abandoning this minor tweak as I am convinced that there is something more fundamentally wrong here.
llvm-dwarfdump doesn't call the right API sequence to show the bug; it requires a DWP, looking up a specific unit by hash via the DWP index, and then scanning all the units. I can envision how to do a unittest for that, but the unittest infrastructure doesn't currently support building a DWP. I can't take the time right now to address this, but I'll keep it on The List.

Tue, Aug 7, 10:47 AM · debug-info

Fri, Aug 3

JDevlieghere closed D50237: [DebugInfo/Verifier] Don't emit error for missing module in index.
Fri, Aug 3, 5:02 AM · debug-info
labath accepted D50237: [DebugInfo/Verifier] Don't emit error for missing module in index.

looks good. If we end up doing this too often, we can consider changing the blacklist to a whitelist.

Fri, Aug 3, 3:50 AM · debug-info
JDevlieghere created D50237: [DebugInfo/Verifier] Don't emit error for missing module in index.
Fri, Aug 3, 3:43 AM · debug-info
jhenderson added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

I just noticed that llvmObject has an object_error equivalent to std::errc, with a make_error_code method allowing us to turn them into std::error_code for use with the new function. In particular, there are unexpected_eof and parser_failed errors, which might be more appropriate for certain situations in this file. What do you think?

Fri, Aug 3, 2:07 AM · debug-info

Thu, Aug 2

probinson updated subscribers of D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.
Thu, Aug 2, 2:07 PM · debug-info
aprantl accepted D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.

Thanks!

Thu, Aug 2, 1:19 PM · debug-info
probinson created D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.
Thu, Aug 2, 12:38 PM · debug-info
probinson added inline comments to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..
Thu, Aug 2, 8:04 AM · debug-info
jhenderson requested changes to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..
Thu, Aug 2, 3:29 AM · debug-info
JDevlieghere accepted D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

In the differential you linked we agreed to have the API include an error code as an argument. That still leaves us the option to pass an inconvertibleErrorCode() as the first argument.

Thu, Aug 2, 2:46 AM · debug-info

Wed, Aug 1

vleschuk added a comment to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..

Ping. Any questions?

Wed, Aug 1, 10:25 PM · debug-info
probinson closed D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Wed, Aug 1, 1:54 PM · debug-info
probinson closed D49743: [DebugInfo/DWARF] [3/4] De-segregate type units and compile units. NFC.
Wed, Aug 1, 1:50 PM · debug-info
probinson closed D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.
Wed, Aug 1, 1:47 PM · debug-info
probinson closed D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.
Wed, Aug 1, 1:44 PM · debug-info
rnk added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.
In D49915#1184590, @vsk wrote:
In D49915#1184574, @rnk wrote:
In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.

Yes, c.f the source-based coverage instrumentation which ended up needing two bits of information from the frontend per segment: http://llvm.org/doxygen/structllvm_1_1coverage_1_1CoverageSegment.html.

(Btw I'm not convinced we have to grow DILocation; I'm not aware of the use case of enabling SamplePGO along with GCOV instrumentation, so I don't see why it's not feasible to borrow 1-2 bits from the discriminator operand. + @xur & @danielcdh for any corrections here)

Wed, Aug 1, 11:29 AM · debug-info
vsk updated subscribers of D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.
In D49915#1184574, @rnk wrote:
In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Right. The frontend should try to encode enough information that the various consumers of DILocations can decide for themselves what tables they should emit. Our line tables should probably stay as they are now, and the various code coverage and instr profiling implementations can do whatever they think is appropriate with this information. With that in mind, if we're going to grow DILocation, what we really want is probably a set of location flags, if we're going to grow DILocation.

Wed, Aug 1, 11:04 AM · debug-info
rnk added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.
In D49915#1184440, @vsk wrote:

A caveat to this: plenty of users (and plenty of tests) expect to be able to set breakpoints on scope-closing curly braces. Assigning line 0 to those braces would at the minimum break a lot of tests, and would likely be viewed as a serious regression.

Wed, Aug 1, 10:56 AM · debug-info
vsk added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.

DILocation isn't only used for coverage, I think this should probably be a more general purpose flag, like "isImplicitCode", to indicate that the code was generated as part of a destructor cleanup, an epilogue, or something else that requires the compiler to generate IR that isn't obviously attributed to user source code. Then GCOV can decide for itself how it wants to generate coverage.

We have a marker for compiler-generated code that has no relation to any source code; it's the special line number 0. This sounds like this patch is trying to work around a frontend bug, and really the frontend shouldn't stick misleading line numbers on the code? There's an RAII object in clang to generate locations for compiler/auto-generated code.

Wed, Aug 1, 9:33 AM · debug-info
aprantl added a comment to D49676: [DWARF] support for .debug_addr (consumer).

It looks like my request wasn't addressed either. Can you please fix this soon?

Wed, Aug 1, 7:54 AM · debug-info
aprantl added a comment to D49915: [IR] Add a boolean field in DILocation to know if a line must covered or not.

DILocation isn't only used for coverage, I think this should probably be a more general purpose flag, like "isImplicitCode", to indicate that the code was generated as part of a destructor cleanup, an epilogue, or something else that requires the compiler to generate IR that isn't obviously attributed to user source code. Then GCOV can decide for itself how it wants to generate coverage.

Wed, Aug 1, 7:48 AM · debug-info
JDevlieghere closed D50057: [MC] Report fatal error for DWARF types for non-ELF object files.
Wed, Aug 1, 5:53 AM · debug-info
jhenderson added a comment to D49676: [DWARF] support for .debug_addr (consumer).

Closed by rL338447

Wed, Aug 1, 2:38 AM · debug-info

Tue, Jul 31

vleschuk closed D50005: [DWARF] Basic support for producing DWARFv5 .debug_addr section.
Tue, Jul 31, 10:48 PM · debug-info
vleschuk closed D49676: [DWARF] support for .debug_addr (consumer).

Closed by rL338447

Tue, Jul 31, 10:22 PM · debug-info