Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (349 w, 1 d)

Recent Activity

Yesterday

probinson added a comment to D72828: [DWARF5] Added support for emission of debug_macro section..

My comments have been addressed, but leaving final approval to @dblaikie.

Fri, Jan 17, 9:25 AM · Restricted Project, debug-info
probinson added a comment to D72900: [DebugInfo] Support 64-bit DWARF for .debug_names..

Some parts of this could be extracted into a separate NFC (or mostly NFC) patch:

  • The Dwarf.h part
  • In DWARFAcceleratorTable.h, merging the Header and HeaderPOD structs, and removing the Padding field. (Adding the Format field would still be part of the functional patch.)
  • In DWARFAcceleratorTable.cpp, eliminating the Padding field, and introducing CommonHeaderSize in DWARFDebugNames::Header::extract()
Fri, Jan 17, 9:05 AM · Restricted Project, debug-info

Thu, Jan 16

probinson added inline comments to D72828: [DWARF5] Added support for emission of debug_macro section..
Thu, Jan 16, 12:56 PM · Restricted Project, debug-info

Tue, Jan 14

probinson added a comment to D71487: [LLDB] Fix address computation for inline function.

BTW: is used to be that both DW_AT_low_pc and DW_AT_high_pc would be set to zero when a function was dead stripped. This was back when both the low and high pc used DW_FORM_addr (a file address). But then DWARF changed such that DW_AT_high_pc could be encoded as a data form: DW_FORM_data1, DW_FORM_data2, DW_FORM_data4, or DW_FORM_data8. This is used to mean it is an offset from the low PC. Seems the linkers now didn't have a relocation for the DW_AT_high_pc so they couldn't zero it out. This is sad because we can end up with many functions at address zero that didn't get linked, and if zero is a valid address, then our DWARF contains a bunch of useless info that only hides which function is the real function for address zero.

Tue, Jan 14, 10:32 AM · Restricted Project
probinson added a comment to D69309: Support template instantiation in the expression evaluator.

Basically, today the debug info will describe an entity named "Foo<int>". The accelerator tables all reference this name. So when Clang asks us if we know "Foo" (which is what happens when instantiating), we fail to find the right instantiations. The consensus of the above discussion was that we should change the debug info to have "Foo" as the name of any instantiation, with a child DIE describing the template arguments. Just doing this in the compiler causes test failures in LLDB, so there's some work to do in LLDB to support this.

Frederic, you say that "doing this in the compiler causes test failures in LLDB", which implies you have tried adding the template in the compiler. Do you have that compiler patch lying around so that we could have a look at what can be done on the lldb side?

I agree that a good long term fix is to have "Foo" as an entity in DWARF, although for backwards compatibility it might be better if the "Foo" template just contained references to the instantiations rather than having them as children.

I am afraid you're overestimating the scope of that idea. I *think* that Fred was referring to simply changing the string that gets put into the DW_AT_name field of the /instantation/ (and, by extension, the accelerator table). The debug info would still describe instantiations only.

Just confirming that this is indeed what I meant. I don't have the patch handy anymore (it was extremely small, about 5 lines IIRC).

Tue, Jan 14, 10:23 AM · Restricted Project
probinson accepted D72469: SCC: Allow ReplaceNode to safely support insertion.

Seems to me we've needed to do this sort of tweak before, so LGTM.
Because it depends on the memory-management behavior of the map, it would be tricky to test with IR input. Would it be feasible to do something in llvm/unittests/ADT/SCCIteratorTest.cpp that could trigger this reliably? Can be done as a follow-up, as I know the branch is coming soon.

Tue, Jan 14, 7:24 AM · Restricted Project

Dec 17 2019

probinson added a comment to D70524: Support DebugInfo generation for auto return type for C++ functions..

"DW_TAG_unspecified_type auto" should be emitted for the function declared/defined as auto returnning. Do you have other test cases in mind, where above points diverges ??

Dec 17 2019, 10:44 AM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols.

@dblaikie let me reflect this back to make sure I get it:
Template members (methods or variables) would never appear in the *metadata* description of the struct; but metadata descriptions of the instances would refer back to that struct (as the scope for the instance). Then DwarfDebug would paste them all together when emitting the DWARF description(s). The in-struct child DIE could then have the deduced type because by the time the definition metadata is produced, we actually know what that type is. This is okay because template data members are necessarily static; they don't affect size or layout of the struct in any way.

Dec 17 2019, 10:44 AM · Restricted Project, debug-info

Dec 16 2019

probinson added a comment to D71508: [DebugInfo] Duplicate file names in debug info.

Do we have a similar problem if the filespec has an embedded ./ or ../ in it?

problems occur only when filespec starts with ./ otherwise it's fine.

Dec 16 2019, 9:20 AM · Restricted Project, debug-info
probinson added a comment to D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols.

I have to wonder if we're not being too eager to produce the debug info. It seems that the undeduced type problem arises because we're trying to produce debug info before we've really finished instantiating value<int> here. But how Clang works pretty much always confuses me.

Dec 16 2019, 7:20 AM · Restricted Project, debug-info
probinson accepted D71546: [MachO] Fix detecting malformed DWARF..

LGTM

Dec 16 2019, 6:52 AM · Restricted Project, lld
probinson added a comment to D71508: [DebugInfo] Duplicate file names in debug info.

Do we have a similar problem if the filespec has an embedded ./ or ../ in it? I'm thinking some broader canonicalization ought to be done here.
$ clang ./dir1/dir2/../dir3/file.c
should resolve to dir1/dir3/file.c shouldn't it?

Dec 16 2019, 6:04 AM · Restricted Project, debug-info

Dec 13 2019

probinson added a comment to D70524: Support DebugInfo generation for auto return type for C++ functions..

Hmm, maybe this feature/suggestion is broken or at least not exactly awesome when it comes to auto-returning functions that are eventually void-returning functions? Now the function definition has no DW_AT_type to override the unspecified_type in the declaration... :/ that's unfortunate (@probinson - thoughts?)

Dec 13 2019, 4:27 AM · debug-info, Restricted Project, Restricted Project

Dec 12 2019

probinson added a comment to D71408: [lit] Remove lit's REQUIRES-ANY directive.

I'd rather see REQUIRES-ANY deprecated/removed, in favor of using expressions in REQUIRES. (REQUIRES-ANY predates expressions.)
IIRC it is used rarely and it would not be a big deal to eliminate it.

Dec 12 2019, 7:18 AM · Restricted Project, Restricted Project, Restricted Project

Dec 11 2019

probinson added inline comments to D71244: [DWARF5][DWARFVerifier] Check that Skeleton compilation unit does not have children..
Dec 11 2019, 8:58 AM · Restricted Project, debug-info

Dec 10 2019

probinson added inline comments to D71244: [DWARF5][DWARFVerifier] Check that Skeleton compilation unit does not have children..
Dec 10 2019, 10:55 AM · Restricted Project, debug-info
probinson added inline comments to D71244: [DWARF5][DWARFVerifier] Check that Skeleton compilation unit does not have children..
Dec 10 2019, 9:41 AM · Restricted Project, debug-info
probinson accepted D71274: [DebugInfo] Fix printing of DW_LNS_set_isa.

LGTM.

Dec 10 2019, 9:23 AM · Restricted Project
probinson added a comment to D71244: [DWARF5][DWARFVerifier] Check that Skeleton compilation unit does not have children..

I like it, but the others should have a chance to say something.

Dec 10 2019, 9:13 AM · Restricted Project, debug-info

Dec 9 2019

probinson accepted D71185: [DWARF5] Start emitting DW_AT_dwo_name when -gdwarf-5 is specified..

Yep LGTM.

Dec 9 2019, 11:11 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D71185: [DWARF5] Start emitting DW_AT_dwo_name when -gdwarf-5 is specified..

Address the inline comments and LGTM.

Dec 9 2019, 10:05 AM · Restricted Project, Restricted Project, debug-info

Dec 6 2019

probinson added a comment to D70524: Support DebugInfo generation for auto return type for C++ functions..

Perhaps we should implement this mode under -fno-standalone-debug (or something more aggressive) since "standalone" is one of the only places I can think of where having the full class definition would be handy

Dec 6 2019, 9:44 AM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D70524: Support DebugInfo generation for auto return type for C++ functions..

@probinson I was reading the C++ "auto" return type issue and I can't come up with a case where we don't have class descriptions across compilation units that are not consistent wrt to auto return type. Do you have a specific example?

The actual return type is known in a compile_unit where the method is defined, and not known in other compile_units. If the non-defining compile_units omit the return type, that means "void" not "auto". That is, one compile unit says it returns "void" and another compile unit says it returns something else. That is the inconsistency I meant.

If we use unspecified_type instead of omitting the return type, then a consumer knows that the method returns *something*, but it will have to look elsewhere to determine what that is.

Yeah, my argument was to omit the declarations of auto-returning functions entirely except in cases where their definition is available (either only when the definition is emitted, or whenever the definition's available so the deduced return type can be provided), same as a template instantiation.

So the class would be missing the declaration of the function in cases where the definition isn't available - same as templates and implicit special members.

Dec 6 2019, 7:15 AM · debug-info, Restricted Project, Restricted Project

Dec 5 2019

probinson added a comment to D70524: Support DebugInfo generation for auto return type for C++ functions..

@probinson I was reading the C++ "auto" return type issue and I can't come up with a case where we don't have class descriptions across compilation units that are not consistent wrt to auto return type. Do you have a specific example?

Dec 5 2019, 1:40 PM · debug-info, Restricted Project, Restricted Project

Dec 2 2019

probinson added a comment to D70696: [DebugInfo] Support to emit debugInfo for extern variables.

For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

FWIW I'd only expect it there with -fstandalone-debug - with -fno-standalone-debug I'd expect this code to rely on the assumption that def.c is also compiled with debug info.

Dec 2 2019, 12:09 PM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D70696: [DebugInfo] Support to emit debugInfo for extern variables.

It is just strange that gcc 7.3.1 (did not test, but maybe later gcc versions as well) emits the debuginfo (encoded in dwarf)
even if the external variable is not used in the current compilation unit. Not sure what is the rationale behind it.

Dec 2 2019, 11:58 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D70696: [DebugInfo] Support to emit debugInfo for extern variables.

For the case:

cat def.c
    int global_var = 2;

def.o should have debug info for the definition of global_var.
For the case:

cat noref.c
    extern int global_var;
    int main() {}

I would not expect to see debug info for the declaration of global_var.
For the case:

cat ref.c
    extern int global_var;
    int main() { return global_var; }

I *do* expect to see debug info for the declaration of global_var.

Dec 2 2019, 11:10 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols.

I guess first I'm confused about why the type would be undeduced in the first place, given that it is actually instantiated.
And if undeduced is correct, wouldn't we rather emit these with DW_TAG_unspecified_type?

Dec 2 2019, 10:52 AM · Restricted Project, debug-info
probinson accepted D70784: [FileCheck] Given multiple -dump-input, prefer most verbose.

LGTM

Dec 2 2019, 9:58 AM · Restricted Project

Nov 27 2019

probinson added a comment to D70784: [FileCheck] Given multiple -dump-input, prefer most verbose.

I know we've had previous differences of opinion on how many combinations to test, and I'll bring it up again. I think we really need:

  • only one case where the two options are the same; that demonstrates a repeated option is not an error.
  • minimum cases showing the higher one gets picked: never/fail, fail/always, always/help. By transitivity we conclude the others are fine.
  • only one case for the other order, showing it's not first (or last) always wins.
  • only two cases of environment/command, showing it's not one or the always wins.

So a total of 7 cases, rather than the 16 you have here.

Nov 27 2019, 1:00 PM · Restricted Project
probinson added a comment to D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map.

The ugly path separator pattern {{(/|\\\\)}} appears in 60+ tests. Can we teach clang and other tools to

  1. accept both / and \ input

In general they do, AFAIK, although it's not feasible in cases where / is the character that introduces an option, which is common on standard Windows utilities.

Nov 27 2019, 10:27 AM · Restricted Project, Restricted Project, Restricted Project

Nov 22 2019

probinson added inline comments to D70524: Support DebugInfo generation for auto return type for C++ functions..
Nov 22 2019, 8:47 AM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D70318: Recover debug intrinsics when killing duplicate or empty basic blocks.

Adrian wrote:

aprantl added a reviewer: jmorse.

Full disclosure, Stephen is also a Sony-ite, and I believe organisation-internal approves are generally frowned on. I'm happy with the general direction and implementation of this patch though.

Nov 22 2019, 8:20 AM · Restricted Project, debug-info
probinson added inline comments to D70597: [PHIEliminate] skip dbg instruction when LowerPHINode.
Nov 22 2019, 6:23 AM · Restricted Project

Nov 21 2019

probinson added a comment to D69462: [DebugInfo]: Support for debug_loclists.dwo section in llvm and llvm-dwarfdump..

Hmm, @aprantl @probinson - I can't find the wording in the DWARF5 spec about the use of loclists_base and rnglists_base in split units. It's unclear if these need to be specified in the split unit, or if they're assumed to be zero (or, technically, zero + sizeof(list header))? Currently LLVM doesn't generate a rnglists_base for split DWARFv5 units (I haven't checked the history on that change - but I might've had a hand in it) & this change is going to propagate that choice & do similarly for loclists_base.

Nov 21 2019, 5:49 AM · Restricted Project, debug-info

Nov 14 2019

probinson committed rGbaacd1891851: Fix up lit's tests to run in a multi-config build environment. (authored by probinson).
Fix up lit's tests to run in a multi-config build environment.
Nov 14 2019, 11:30 AM
probinson closed D70239: Fix up lit's tests to run in a multi-config environment.
Nov 14 2019, 11:30 AM · Restricted Project
probinson added a comment to D70239: Fix up lit's tests to run in a multi-config environment.

Done. Makes sense to avoid future mishaps.

Nov 14 2019, 9:02 AM · Restricted Project
probinson updated the diff for D70239: Fix up lit's tests to run in a multi-config environment.

Move the empty lit.cfg to Inputs; undo selecting.py change.

Nov 14 2019, 9:02 AM · Restricted Project
probinson added a comment to D70105: [lit] Better/earlier errors for empty runs.

I think it's using utils\lit\tests\lit.site.cfg, which produces the error, when it would normally use utils/lit/tests/Inputs/nonexistent/lit.cfg. Creating even an empty version of that file would probably fix this.

Nov 14 2019, 6:08 AM · Restricted Project
probinson created D70239: Fix up lit's tests to run in a multi-config environment.
Nov 14 2019, 6:08 AM · Restricted Project

Nov 13 2019

probinson added a comment to D70105: [lit] Better/earlier errors for empty runs.

This patch fails on Windows when built using the Visual Studio 2017 builder. The new RUN lines in selecting.py for the "nonexistent" case will produce a diagnostic complaining about a missing 'build_mode' substitution:

$ ":" "RUN: at line 7"
$ "not" "env" "-u" "FILECHECK_OPTS" "-u" "FILECHECK_DUMP_INPUT_ON_FAILURE" "C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64\python.exe" "D:/Data/upstream/llvm-project/llvm\utils\lit\lit.py" "D:/Data/upstream/mbuild/utils/lit/tests\Inputs/nonexistent"
$ "FileCheck" "--check-prefix=CHECK-BAD-PATH" "D:\Data\upstream\mbuild\utils\lit\tests\selecting.py"
# command stderr:
D:\Data\upstream\mbuild\utils\lit\tests\selecting.py:9:19: error: CHECK-BAD-PATH: expected string not found in input
# CHECK-BAD-PATH: Did not disover any tests for provided path(s).
                  ^
<stdin>:1:1: note: scanning from here
lit.py: D:\Data\upstream\mbuild\utils\lit\tests\lit.site.cfg:18: fatal: unable to find 'build_mode' parameter, use '--param=build_mode=VALUE'
^
<stdin>:1:62: note: possible intended match here
lit.py: D:\Data\upstream\mbuild\utils\lit\tests\lit.site.cfg:18: fatal: unable to find 'build_mode' parameter, use '--param=build_mode=VALUE'
                                                             ^
Nov 13 2019, 6:03 PM · Restricted Project

Nov 12 2019

probinson accepted D70033: Add a shim for setenv on PS4 since it does not exist on PS4.

LGTM

Nov 12 2019, 2:03 PM · Restricted Project, Restricted Project

Nov 7 2019

probinson added a comment to D69955: [DebugInfo] Add helper for finding entry value candidates [NFC].

Please remember to provide full-context diffs.

Nov 7 2019, 11:25 AM · Restricted Project

Nov 6 2019

probinson added a comment to D69886: [DebugInfo] Support for DW_OP_implicit_pointer (CodeGen phase).

I'd expect more than 3 patches and in this order:

Nov 6 2019, 4:52 PM · debug-info, Restricted Project
probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

Sigh, one last typo. I'm happy otherwise.

Nov 6 2019, 3:53 PM · Restricted Project
probinson added a comment to D69893: libunwind: Evaluating DWARF operation DW_OP_pick is broken.

Is it tested? Intuitively I would expect DW_OP_pick to be kind of an unusual operator, unlikely to be seen in the wild.

Nov 6 2019, 11:32 AM · Restricted Project
probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

Just a couple of typos, and I'm happy. I agree with the other reviewers on the last needed test tweaks.

Nov 6 2019, 9:22 AM · Restricted Project

Nov 5 2019

probinson added inline comments to D69822: [clang] Add new -fdebug-default-version flag..
Nov 5 2019, 1:15 PM · Restricted Project
probinson added inline comments to D69822: [clang] Add new -fdebug-default-version flag..
Nov 5 2019, 1:15 PM · Restricted Project
probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

The test is in the right place, now it needs to behave more like other driver tests. Sorry if it feels like I'm whaling on you, but the driver is a bit of a peculiar beast with an atypical testing mode. Taming it is harder than it looks.

Nov 5 2019, 1:06 PM · Restricted Project
probinson added inline comments to D69822: [clang] Add new -fdebug-default-version flag..
Nov 5 2019, 7:14 AM · Restricted Project

Nov 4 2019

probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

The maze of twisty -g passages gets a new secret door. Oh well.

If you find this to be a significant complication, I'd really like to discuss it further - I know there are some twisty things (one of the worst I created when it comes to -fsplit-dwarf-inlining, -gsplit-dwarf, -gmlt combinations) - but this seems pretty simple/tidy/orthogonal to me.

Nov 4 2019, 3:21 PM · Restricted Project
probinson added inline comments to D69822: [clang] Add new -fdebug-default-version flag..
Nov 4 2019, 3:12 PM · Restricted Project
probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

Want to decouple setting the DWARF version from enabling/disabling generation of debug info.

Um, why?

If you've got a big build system with various users opting in/out of DWARF and you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to your build system, because that'd turn on debug info in cases that don't need it - but it's easier to change the default than to modify all cases that enable dwarf across the codebase.

Open to discussion about whether this is a good/bad idea - but it'd be useful for Google at least & seemed low-cost enough to go this route.

Nov 4 2019, 2:53 PM · Restricted Project
probinson added inline comments to D69822: [clang] Add new -fdebug-default-version flag..
Nov 4 2019, 2:44 PM · Restricted Project
probinson added a comment to D69822: [clang] Add new -fdebug-default-version flag..

Want to decouple setting the DWARF version from enabling/disabling generation of debug info.

Nov 4 2019, 2:29 PM · Restricted Project

Nov 1 2019

probinson updated subscribers of D69393: [RFC][DebugInfo] emit user specified address_space in dwarf.

During experimenting with linux kernel codes, I found that clang does not allow address_space attribute for function pointers, specifically, in clang/lib/Sema/SemaType.cpp,

// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall not be
// qualified by an address-space qualifier."
if (Type->isFunctionType()) {
  S.Diag(Attr.getLoc(), diag::err_attribute_address_function_type);
  Attr.setInvalid();
  return;
}

But linux kernel tries to annotate signal handling function pointer with address space to indicate it is accessing user space.

typedef __signalfn_t __user *__sighandler_t;
typedef __restorefn_t __user *__sigrestore_t;

Such attribute makes sense for linux since indeed the signal handler code resides in user space and the kernel pointer
merely points to user memory here.

But such attributes are not allowed for function pointers.

Maybe somebody can give some context about this particular ISO/IEC TR 18037 specification? cc @probinson

Nov 1 2019, 9:29 AM · Restricted Project, debug-info

Oct 25 2019

probinson added a comment to D69393: [RFC][DebugInfo] emit user specified address_space in dwarf.
In D69393#1720816, @ast wrote:

The address spaces envisioned by the Linux kernel appear to be more informational and not descriptive of hardware characteristics.

From the kernel pov the __user and normal are two different address spaces that must be accessed via different kernel primitives.
User access needs stac/clac on x86 and other precautions.
iirc other architectures have co-processor address space that needs its own load/store equivalents.
__percpu is also different address space. It's roughly equivalent to __thread in user space.

Oct 25 2019, 2:10 PM · Restricted Project, debug-info

Oct 24 2019

probinson added a comment to D69393: [RFC][DebugInfo] emit user specified address_space in dwarf.

The DW_AT_address_class attribute is intended to be used for target architectures where a simple address value is ambiguous, and the debugger needs additional information to resolve the ambiguity. The canonical example is something like i386 with its segmented addresses, and an address value has different interpretations depending on whether it is (for example) a "near" or "far" pointer.
This would be in contrast to something like the VAX, where the upper two bits of the address identify a subdivision of the flat address space (VAX calls these P0, P1, S0, where P0/P1 are Process [user] spaces and S0 is system/kernel space); however, there is no ambiguity because addresses are always 32-bit and the same address value can't be interpreted different ways.

Oct 24 2019, 6:31 PM · Restricted Project, debug-info
probinson added a comment to D67723: [DebugInfo] Add option to disable inline line tables..

These experiments are convincing me that, in general, line zero isn't that helpful for DWARF consumers. If the goal is to get smooth stepping, we may want to refocus on getting reliable is_stmt bits in the line table.

Oct 24 2019, 2:40 PM · debug-info, Restricted Project, Restricted Project

Oct 16 2019

probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

(@dblaikie Aside regarding noreturn, the original DWARF proposal was for a debugger wanting to know a given function will not return.

I'd still be curious to know which consumer, if they're still interested, what feature they're trying to build with it, if they're using Clang/LLVM's output, etc.

Oct 16 2019, 2:48 PM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

Oct 16 2019, 8:27 AM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D69028: [DebugInfo] Correctly place DW_OP_derefs for arguments passed on stack.

Just to clarify things in my own mind: dbg.declare expressions describe a location, while dbg.value expressions describe a value? And that's why you're wanting to add a trailing deref?
Then when we emit the actual DWARF, the trailing deref is omitted so the expression again describes a location.

Oct 16 2019, 7:59 AM · Restricted Project

Oct 15 2019

probinson added a comment to D67723: [DebugInfo] Add option to disable inline line tables..

Apologies for missing this until now. Our email system keeps dropping stuff sent by Phabricator.

Oct 15 2019, 2:14 PM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

Hi David,
I did some digging about DW_AT_defaulted and stuff, not much of a success but. Here's what I found -- http://dwarfstd.org/Issues.php?type=closed4 -- here it;s still marked as open, not sure what that means. Abbreviations on this page doesn't describe what "open" meant. But since, it's already in DWARF5 Spec -- it must be accepted.

Oct 15 2019, 9:52 AM · debug-info, Restricted Project, Restricted Project

Oct 9 2019

probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Oct 9 2019, 4:21 PM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D68620: DebugInfo: Use base address selection entries for debug_loc.

I'll ask re Sony debugger. I have no direct visibility to that code.

Oct 9 2019, 10:56 AM · Restricted Project

Oct 8 2019

probinson added a comment to D68270: DWARFDebugLoc: Add a function to get the address range of an entry.

Do we care whether llvm-dwarfdump's output bears any similarities to the output from GNU readelf or objdump? There has been a push lately to get the LLVM "binutils" to behave more like GNU's, although AFAIK it hasn't gotten to the DWARF dumping part.

Oct 8 2019, 11:16 AM · Restricted Project
probinson added inline comments to D68639: [MachineScheduler] Add a flag to enable scheduling of cfi instructions.
Oct 8 2019, 11:07 AM · Restricted Project
probinson requested changes to D68620: DebugInfo: Use base address selection entries for debug_loc.

For some reason a previous comment caused it to set Accept, and the only way I know to undo it is to set Request Changes. Sorry about that.

Oct 8 2019, 10:08 AM · Restricted Project
probinson added a comment to D68620: DebugInfo: Use base address selection entries for debug_loc.

BTW the BinaryFormat part LGTM and can go in on its own if you like. Should have been done that way in the first place.

Oct 8 2019, 10:06 AM · Restricted Project
probinson accepted D68620: DebugInfo: Use base address selection entries for debug_loc.

Anyone know whether they have consumers (LLDB, the Sony debugger) that would need to be updated for either the v4 changes (use of base address specifiers in classic debug_loc lists) or v5 (base_addressx, etc, etc)?

Oct 8 2019, 9:23 AM · Restricted Project
probinson added a comment to D68465: [DebugInfo] Trim call-clobbered location list entries when tuning for GDB.

@dblaikie I'm also not clear what you're suggestion about .debug_addr entry plus offset. DW_LLE_offset_pair does this, derived from the base address, which ought to be available for any given function, assuming DWARF v5. Can you explain more clearly what's missing?

Right - for loclists there's no need for new forms, etc. It was specifically related to the other review related to this that modifies the in-memory representation of the debug_addr (llvm::AddressPool) - which I assume meant a difference in output in the address pool, but seems it doesn't add the offset inside the pool (but may end up with redundant entries in the pool which should be fixed in any case).

Oct 8 2019, 9:04 AM · Restricted Project, debug-info

Oct 7 2019

probinson added a comment to D68465: [DebugInfo] Trim call-clobbered location list entries when tuning for GDB.

Debugger tuning should not be used directly this way. There should be a DwarfDebug flag, and a CL option, and the default set in the DwarfDebug ctor based on tuning. This allows the defaulting to work how you want, but can be overridden easily for experimentation and testing. There are lots of examples of doing this in the ctor already. Also, if it turns out some other debugger also needs this, it's trivial to fix up the ctor to handle it with no code changes needed elsewhere.

Oct 7 2019, 9:20 AM · Restricted Project, debug-info

Oct 4 2019

probinson added inline comments to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..
Oct 4 2019, 12:55 PM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D68135: [lit] Set the target-windows feature for any windows environment.

I'm happy to be wrong about the win32 part. And eliminating 'target-windows' in favor of using just 'windows' SGTM.

Oct 4 2019, 12:48 PM · Restricted Project
probinson added inline comments to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..
Oct 4 2019, 12:43 PM · debug-info, Restricted Project, Restricted Project
probinson added a comment to D68425: [NFC] [FileCheck] Fix init of objects in unit tests.

Ok to commit this revised change? I double checked that I ran the unit tests this time and all tests pass.

Oct 4 2019, 8:12 AM · Restricted Project
probinson added a comment to D68135: [lit] Set the target-windows feature for any windows environment.

Updated, with a slightly different form of the regex, that also allows the triple to just end at -windows.

Alternatively, we could just remove this part of the lit config altogether. There's no other OSes that have target-<os> as a feature (only system-<os> for the host where the test is running), and after D68133 and D68136, no tests actually use this feature any longer.

Oct 4 2019, 7:56 AM · Restricted Project

Oct 1 2019

probinson added inline comments to D67649: [FileCheck] Move private interface to its own header.
Oct 1 2019, 7:59 AM · Restricted Project

Sep 30 2019

probinson committed rGed1f3f36aeee: [SSP] [3/3] cmpxchg and addrspacecast instructions can now trigger stack… (authored by probinson).
[SSP] [3/3] cmpxchg and addrspacecast instructions can now trigger stack…
Sep 30 2019, 8:15 AM
probinson committed rL373220: [SSP] [3/3] cmpxchg and addrspacecast instructions can now.
[SSP] [3/3] cmpxchg and addrspacecast instructions can now
Sep 30 2019, 8:09 AM
probinson closed D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Sep 30 2019, 8:09 AM · Restricted Project
probinson committed rG527815f5b087: [SSP] [2/3] Refactor an if/dyn_cast chain to switch on opcode. NFC (authored by probinson).
[SSP] [2/3] Refactor an if/dyn_cast chain to switch on opcode. NFC
Sep 30 2019, 8:09 AM
probinson committed rL373219: [SSP] [2/3] Refactor an if/dyn_cast chain to switch on opcode. NFC.
[SSP] [2/3] Refactor an if/dyn_cast chain to switch on opcode. NFC
Sep 30 2019, 8:09 AM
probinson closed D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Sep 30 2019, 8:09 AM · Restricted Project
probinson committed rG14945186c28e: [SSP] [1/3] Revert "StackProtector: Use PointerMayBeCaptured" "Captured" and… (authored by probinson).
[SSP] [1/3] Revert "StackProtector: Use PointerMayBeCaptured" "Captured" and…
Sep 30 2019, 8:04 AM
probinson added a reverting change for rGf29366b1f594: StackProtector: Use PointerMayBeCaptured: rG14945186c28e: [SSP] [1/3] Revert "StackProtector: Use PointerMayBeCaptured" "Captured" and….
Sep 30 2019, 8:04 AM
probinson committed rL373216: [SSP] [1/3] Revert "StackProtector: Use PointerMayBeCaptured".
[SSP] [1/3] Revert "StackProtector: Use PointerMayBeCaptured"
Sep 30 2019, 7:59 AM
probinson closed D67842: SSP: [1/3] revert r363169.
Sep 30 2019, 7:59 AM · Restricted Project

Sep 27 2019

probinson added a comment to D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.

Ping

Sep 27 2019, 11:37 AM · Restricted Project

Sep 23 2019

probinson committed rL372639: Request commit access for probinson.
Request commit access for probinson
Sep 23 2019, 11:32 AM
probinson updated the diff for D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.

HasAddressTaken now defaults to true; this means explicitly handling innocuous instructions.

  • load does not store anything.
  • atomicrmw cannot store a pointer unless it goes through ptrtoint, which we handle explicitly.
  • ret is irrelevant because the current frame goes away after the ret.
Sep 23 2019, 8:49 AM · Restricted Project
probinson added inline comments to D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Sep 23 2019, 6:31 AM · Restricted Project

Sep 20 2019

probinson added a child revision for D67842: SSP: [1/3] revert r363169: D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Sep 20 2019, 10:47 AM · Restricted Project
probinson added a parent revision for D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC: D67842: SSP: [1/3] revert r363169.
Sep 20 2019, 10:47 AM · Restricted Project
probinson added a child revision for D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC: D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Sep 20 2019, 10:47 AM · Restricted Project
probinson added a parent revision for D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection: D67844: SSP: [2/3] Refactor an if-dyn_cast chain to switch on opcode. NFC.
Sep 20 2019, 10:47 AM · Restricted Project
probinson created D67845: SSP: [3/3] cmpxchg and addrspacecast can now trigger stack protection.
Sep 20 2019, 10:44 AM · Restricted Project