Re: [lldb-dev] [cfe-dev] [RFC] LLVM bug lifecycle BoF - triaging

[Kristof Beyls via lldb-dev]

Hi Tamas,

Thanks for highlighting this - it shows that at least we should have a 
description of what it means for a bug to be assigned to someone. Your 
interpretation of a bug being “locked” doesn’t sound unreasonable to me without 
any other description being available.

For the clang static analyser component, it is configured in bugzilla that when 
a new bug is raised against it, there is a default assignee. My guess is that 
most bugs reported against that component just keep on having that default 
assignee.
I think it’d be an improvement to move default assignees (for the components 
that have them) to the “default cc” list. That way, the same people still get 
notified when a bug is raised, but bugs don’t get automatically assigned to 
them. It’ll take an actual conscious action to assign a bug - so hopefully a 
bug being assigned to someone can become more meaningful. Or in short, a setup 
that is somewhat similar to what you describe the LibreOffice project has 
indeed seems better.

Thanks!

Kristof


On 6 Oct 2018, at 22:53, Tamás Zolnai 
mailto:zolnaitamas2...@gmail.com>> wrote:

Hi all,

Just a short feedback about my first impression of the llvm bugzilla. I just 
requested an account for bugzilla and I just did some browsing the bugs. I 
checked static analyzer related bugs and as I see almost all bugs are assigned, 
which is a bit strange to me. Also most of the assigned bugs were assigned to 
two assignees, so I expect that these two people don't actually work on that 
~600 bugs.

So I'm a bit confused now what assigning means in llvm bugzilla. In general for 
me assigning means the bug is "locked", somebody is working on that issue, so I 
should not pick it up for working on it. Which means that by now almost every 
static analyzer related bugs are locked in bugzilla, so I need to find a task 
somewhere else.

In LibreOffice project we also use bugzilla and only assign a bug if the 
assignee is actually working on that issue or he/she is planning to work on it 
soon. Also we reset assignee back to "non" after some months of inactivity. 
Which means that most of the bugs are unassinged so new contributors can pick 
them up (also can filter for unassigned bugs). If a bug is related to an area 
which has an "owner" or expert, we add the expert to the "CC" list so he/she 
get notified.

I did not find any information about that what assigning means in llvm 
bugzilla, so I don't know whether it have a different meaning what I expected 
and described above.

Best Regards,
Tamás Zolnai


Kristof Beyls via cfe-dev 
mailto:cfe-...@lists.llvm.org>> ezt írta (időpont: 
2018. okt. 4., Cs, 11:55):
Hi all,

I’d like to share a few thoughts and analysis results on the LLVM bug life 
cycle, especially the reporting/triaging part.

As one of the few people creating llvm bugzilla accounts when people request an 
account, I started to have a feel that many reported bugs, especially by 
first-time reporters, never get any reply or feedback, let alone be acted on.
If people go through the effort of requesting an account, and then reporting 
the bug, they show motivation to contribute to the project. However, if then 
they see zero return on their effort spent, even if it’s just a confirmation of 
the bug indeed being real or an explanation of what they thought to be a bug 
isn’t actually a bug, I fear as a community we disincentify a large number of 
potential long-term contributors.

The above was all based on gut feel, so I tried to gather a bit more data to 
see if my feel was correct or not.
I scraped the bugs in bugzilla and post-processed them a bit. Below is a chart 
showing, year by year, how long it takes for a reported bug to get any comment 
from anyone besides to original reporter. If the bug is still open and didn’t 
have any reaction after half a year the chart categorizes is as an “infinite” 
response time.

 [X]
It shows that in recent years the chance of never getting a response to a bug 
report has been increasing.
For some bugs - e.g. an experienced LLVM developer records a not-that-important 
bug in bugzilla - that may be just fine.
However, I assume that for people reporting a bug for the first time, the 
majority may look at least for confirmation that what they reported is actually 
a bug.
The chart shows (blue bars) that about 50% of first-time bug reporters never 
get any reply.

I also plotted which components get the most reported bugs that don’t get any 
reaction and remain open:
[X]
The percentage at the top of the bars is the percentage of bugs against that 
component that never get any reaction. The bar height shows the absolute 
numbers.


I hope that at the “Lifecycle of LLVM bug reports” BoF at the upcoming dev 
meeting in San Jose (https://llvmdev18.sched.com/event/H2T3, 17th of October, 
10.30am), we can discuss what could be done to improve the experience for 
first-time reporters and also to reduce the number of bug reports that 
seemingly get ignored completely.
By sen

Re: [lldb-dev] [cfe-dev] [RFC] LLVM bug lifecycle BoF - triaging

[Kristof Beyls via lldb-dev]

On 6 Oct 2018, at 23:50, Alex Rønne Petersen 
mailto:a...@alexrp.com>> wrote:

Hello,

I am not a frequent poster on the LLVM mailing lists, but I happened to notice 
this thread and thought I'd weigh in.

Over 2 years ago, I reported this bug: 
https://bugs.llvm.org/show_bug.cgi?id=29102

We had to add a pretty ugly workaround in Mono to deal with that, and the 
workaround is still to this day written to apply to *all* Clang versions on 
ARM64 because we've gotten no response to the bug. This is what we're doing 
currently: https://github.com/mono/mono/blob/master/mono/utils/atomic.h#L209

I think this looks to be a pretty serious bug that shouldn't have gone 
unacknowledged for so long. If there had been any kind of response to the bug, 
I would've even been happy to cook up a patch. But, frankly, without any 
confirmation that a bug is valid, very few potential contributors are going to 
put in the time and effort to write and submit a patch and risk that it gets 
rejected because the issue it's trying to address isn't even considered a bug 
by the project maintainers.

Don't get me wrong, though - I understand from experience that "triage all the 
bugs" is much easier said than done, especially in an open source project. I 
just wanted to back up Kristof's feeling that the project is losing potential 
contributions with a concrete example of such, for what it's worth.

Thank you very much, Alex. I assume that most people who reported a bug and 
never got a reaction on it may not have seen this mail thread. It’s comforting 
to know that at least some people are in a situation like you describe above. 
And therefore, improving the bug lifecycle should increase getting 
contributions from “fresh blood”.


Regards,
Alex

On Thu, Oct 4, 2018 at 11:55 AM Kristof Beyls via cfe-dev 
mailto:cfe-...@lists.llvm.org>> wrote:
Hi all,

I’d like to share a few thoughts and analysis results on the LLVM bug life 
cycle, especially the reporting/triaging part.

As one of the few people creating llvm bugzilla accounts when people request an 
account, I started to have a feel that many reported bugs, especially by 
first-time reporters, never get any reply or feedback, let alone be acted on.
If people go through the effort of requesting an account, and then reporting 
the bug, they show motivation to contribute to the project. However, if then 
they see zero return on their effort spent, even if it’s just a confirmation of 
the bug indeed being real or an explanation of what they thought to be a bug 
isn’t actually a bug, I fear as a community we disincentify a large number of 
potential long-term contributors.

The above was all based on gut feel, so I tried to gather a bit more data to 
see if my feel was correct or not.
I scraped the bugs in bugzilla and post-processed them a bit. Below is a chart 
showing, year by year, how long it takes for a reported bug to get any comment 
from anyone besides to original reporter. If the bug is still open and didn’t 
have any reaction after half a year the chart categorizes is as an “infinite” 
response time.

 [cid:DC7C978D-FC04-470F-BAAE-CC5C623999F0]
It shows that in recent years the chance of never getting a response to a bug 
report has been increasing.
For some bugs - e.g. an experienced LLVM developer records a not-that-important 
bug in bugzilla - that may be just fine.
However, I assume that for people reporting a bug for the first time, the 
majority may look at least for confirmation that what they reported is actually 
a bug.
The chart shows (blue bars) that about 50% of first-time bug reporters never 
get any reply.

I also plotted which components get the most reported bugs that don’t get any 
reaction and remain open:
[cid:130482D2-6DEF-4796-84EC-2968F16B635C]
The percentage at the top of the bars is the percentage of bugs against that 
component that never get any reaction. The bar height shows the absolute 
numbers.


I hope that at the “Lifecycle of LLVM bug reports” BoF at the upcoming dev 
meeting in San Jose (https://llvmdev18.sched.com/event/H2T3, 17th of October, 
10.30am), we can discuss what could be done to improve the experience for 
first-time reporters and also to reduce the number of bug reports that 
seemingly get ignored completely.
By sending this email, I hope to trigger discussion before the BoF, both by 
attendees and non-attendees, so that we have a more fruitful outcome.

At first sight, to me, it seems that the following actions would help:

  *   Let’s introduce some form of “triaged” state in bugzilla, to represent 
that a bug report has been accepted as describing a real problem; able to be 
acted on (e.g. has a suitable reproducer); and not being a duplicate of another 
bug report. Looking at 
https://bugzilla.readthedocs.io/en/5.0/using/editing.html#life-cycle-of-a-bug, 
maybe the best way to achieve this would be for newly raised bugs to by default 
go to an “UNCONFIRMED” state instead of “NEW”? Moving the status to “NEW” or 
“CONFIRME

[lldb-dev] DW_OP_deref handling

[Davide Italiano via lldb-dev]

Hi Greg,
I have this issue that I suspect is a bug in lldb’s handling of DW_OP_deref.
I wasn’t able to craft an easy C++ testcase, so I’ll start from my
motivating (swift) example.

Given the following code:

func use(_ t : T) {}
func single(_ t : T) {
  let x = t
  use(x)
}
let s = "hello"
single(s)

The IR emitted for the local binding `x` in `single` is an alloca
containing the address of `x`. Hence, the DWARF, correctly contains a
DW_OP_deref expression, i.e.:

0x00da: TAG_variable [9]
 AT_location( fbreg -16, deref )
 AT_name( "x" )
 AT_decl_file(
"/Users/dcci/work/swift/build/Ninja-RelWithDebInfoAssert+stdlib-RelWithDebInfo/swift-macosx-x86_64/bin/pat.swift"
)
 AT_decl_line( 4 )
 AT_type( {0x0204} ( $sxD ) )

When I debug this with lldb, I expect lldb to print the value that’s
pointed by %rbp - 16,
i.e.
(lldb) register read $rbp
 rbp = 0x7ffeefbff860

(lldb) mem read 0x7ffeefbff850
0x7ffeefbff850: 10 f8 bf ef fe 7f 00 00 b8 32 4d 00 01 00 00 00
....?2M.
0x7ffeefbff860: c0 f8 bf ef fe 7f 00 00 64 09 00 00 01 00 00 00
?...d...

So, the value that I expected to be printed is 0x7ffeefbff810.

What I instead see in the debugger is:
(T) x = 

The value `0xe500` happens to be the value that’s pointed
by the address 0x7ffeefbff810, that is:

(lldb) mem read 0x7ffeefbff810
0x7ffeefbff810: 00 00 00 00 00 00 00 e5 68 65 6c 6c 6f 00 00 00
...?hello...
0x7ffeefbff820: 05 00 00 00 00 00 00 00 40 32 4d 00 01 00 00 00
@2M.

Is this an expected behavior? I did expect DW_OP_deref to just do a
single dereference and not two. It looks like when we call
`UpdateValue()` in lldb we do have the correct value in `m_value`, but
GetPointerValue() ends up printing the value that’s *pointed by*
what’s in `m_value`, i.e.

m_integer = {
  U = {
VAL = 0x7ffeefbff810
pVal = 0x7ffeefbff810
  }

as this is a LoadAddress. Greg, as you wrote this code (and touched it
last), what do you expect to be the correct semantic here?

Removing a dereference from DW_OP_deref in the DWARFASTParser works
around the issue, but I’m fairly sure it’s the incorrect fix. Are we
maybe setting the value type incorrectly?


Best,


—

Davide
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] DW_OP_deref handling

[Greg Clayton via lldb-dev]

The issue is DW_OP_deref dereferences a pointer and pushes it onto the stack. 
The code currently, for a load address that is on top of the stack does:

Value::ValueType value_type = stack.back().GetValueType();
switch (value_type) {
case Value::eValueTypeLoadAddress:
  if (exe_ctx) {
if (process) {
  lldb::addr_t pointer_addr = 
stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
  Status error;
  lldb::addr_t pointer_value = process->ReadPointerFromMemory(pointer_addr, 
error);
  if (pointer_value != LLDB_INVALID_ADDRESS) {
stack.back().GetScalar() = pointer_value;
stack.back().ClearContext();
  } else {
return false;
  }

Here we just use "stack.back()" to access the item at the top of the expression 
value stack. The top value has a Value::ValueType of 
Value::eValueTypeLoadAddress. We don't reset this value to be 
Value::eValueTypeScalar, but we probably should and that will probably fix this 
issue. 

DWARF5 finally added the ability to track what each value means on the 
expression stack. Prior to DWARF 5, we had no idea what each entry on the 
expression value stack was (file address, load address 
(Value::eValueTypeLoadAddress), plain value (Value::eValueTypeScalar). We have 
tracked this info for a while now, but the DWARF5 spec is much more specific on 
how things should be treated. From the spec:

DW_OP_deref

The DW_OP_deref operation pops the top stack entry and treats it as an address. 
The popped value must have an integral type. The value retrieved from that 
address is pushed, and has the generic type. The size of the data retrieved 
from the dereferenced address is the size of an address on the target machine.

No the "The value retrieved from that address is pushed, and has the generic 
type." statement. In LLDB this means the value's ValueType should be set to 
Value::eValueTypeScalar.

So try modifying the code with 
stack.back().SetValueType(Value::eValueTypeScalar) after the 
stack.back().ClearContext():

  if (pointer_value != LLDB_INVALID_ADDRESS) {
stack.back().GetScalar() = pointer_value;
stack.back().ClearContext();
stack.back().SetValueType(Value::eValueTypeScalar);
  } else {
return false;
  }

That might fix things.

The way the expression ends up right now we end up with an expression stack 
saying that the value is a load address which means we must read the value from 
memory in order to find it. This would be the right thing to do if your 
location expression was:

0x00da: TAG_variable [9]
 AT_location( fbreg -16 )
 AT_name( "x" )
 AT_decl_file(...)
 AT_decl_line( 4 )
 AT_type( {0x0204} ( $sxD ) )

Note I removed the "deref" from the location expression. In this case the 
variable would need to be read from memory. If the value was to be in a 
register, then the location would be:

0x00da: TAG_variable [9]
 AT_location( reg16 )
 AT_name( "x" )
 AT_decl_file(...)
 AT_decl_line( 4 )
 AT_type( {0x0204} ( $sxD ) )

Then the value would be in register 16 itself (not dereferenced).

Hope this all makes sense. If you have any questions let me know.

Greg


> On Oct 11, 2018, at 10:52 AM, Davide Italiano  wrote:
> 
> Hi Greg,
> I have this issue that I suspect is a bug in lldb’s handling of DW_OP_deref.
> I wasn’t able to craft an easy C++ testcase, so I’ll start from my
> motivating (swift) example.
> 
> Given the following code:
> 
> func use(_ t : T) {}
> func single(_ t : T) {
>  let x = t
>  use(x)
> }
> let s = "hello"
> single(s)
> 
> The IR emitted for the local binding `x` in `single` is an alloca
> containing the address of `x`. Hence, the DWARF, correctly contains a
> DW_OP_deref expression, i.e.:
> 
> 0x00da: TAG_variable [9]
> AT_location( fbreg -16, deref )
> AT_name( "x" )
> AT_decl_file(
> "/Users/dcci/work/swift/build/Ninja-RelWithDebInfoAssert+stdlib-RelWithDebInfo/swift-macosx-x86_64/bin/pat.swift"
> )
> AT_decl_line( 4 )
> AT_type( {0x0204} ( $sxD ) )
> 
> When I debug this with lldb, I expect lldb to print the value that’s
> pointed by %rbp - 16,
> i.e.
> (lldb) register read $rbp
> rbp = 0x7ffeefbff860
> 
> (lldb) mem read 0x7ffeefbff850
> 0x7ffeefbff850: 10 f8 bf ef fe 7f 00 00 b8 32 4d 00 01 00 00 00
> ....?2M.
> 0x7ffeefbff860: c0 f8 bf ef fe 7f 00 00 64 09 00 00 01 00 00 00
> ?...d...
> 
> So, the value that I expected to be printed is 0x7ffeefbff810.
> 
> What I instead see in the debugger is:
> (T) x = 
> 
> The value `0xe500` happens to be the value that’s pointed
> by the address 0x7ffeefbff810, that is:
> 
> (lldb) mem read 0x7ffeefbff810
> 0x7ffeefbff810: 00 00 00 00 00 00 00 e5 68 65 6c 6c 6f 00 00 00
> ...?h

Re: [lldb-dev] DW_OP_deref handling

[Davide Italiano via lldb-dev]

On Thu, Oct 11, 2018 at 11:16 AM Greg Clayton  wrote:
>
> The issue is DW_OP_deref dereferences a pointer and pushes it onto the stack. 
> The code currently, for a load address that is on top of the stack does:
>
> Value::ValueType value_type = stack.back().GetValueType();
> switch (value_type) {
> case Value::eValueTypeLoadAddress:
>   if (exe_ctx) {
> if (process) {
>   lldb::addr_t pointer_addr = 
> stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
>   Status error;
>   lldb::addr_t pointer_value = 
> process->ReadPointerFromMemory(pointer_addr, error);
>   if (pointer_value != LLDB_INVALID_ADDRESS) {
> stack.back().GetScalar() = pointer_value;
> stack.back().ClearContext();
>   } else {
> return false;
>   }
>
> Here we just use "stack.back()" to access the item at the top of the 
> expression value stack. The top value has a Value::ValueType of 
> Value::eValueTypeLoadAddress. We don't reset this value to be 
> Value::eValueTypeScalar, but we probably should and that will probably fix 
> this issue.
>
> DWARF5 finally added the ability to track what each value means on the 
> expression stack. Prior to DWARF 5, we had no idea what each entry on the 
> expression value stack was (file address, load address 
> (Value::eValueTypeLoadAddress), plain value (Value::eValueTypeScalar). We 
> have tracked this info for a while now, but the DWARF5 spec is much more 
> specific on how things should be treated. From the spec:
>
> DW_OP_deref
>
> The DW_OP_deref operation pops the top stack entry and treats it as an 
> address. The popped value must have an integral type. The value retrieved 
> from that address is pushed, and has the generic type. The size of the data 
> retrieved from the dereferenced address is the size of an address on the 
> target machine.
>
> No the "The value retrieved from that address is pushed, and has the generic 
> type." statement. In LLDB this means the value's ValueType should be set to 
> Value::eValueTypeScalar.
>
> So try modifying the code with 
> stack.back().SetValueType(Value::eValueTypeScalar) after the 
> stack.back().ClearContext():
>
>   if (pointer_value != LLDB_INVALID_ADDRESS) {
> stack.back().GetScalar() = pointer_value;
> stack.back().ClearContext();
> stack.back().SetValueType(Value::eValueTypeScalar);
>   } else {
> return false;
>   }
>
> That might fix things.
>
> The way the expression ends up right now we end up with an expression stack 
> saying that the value is a load address which means we must read the value 
> from memory in order to find it. This would be the right thing to do if your 
> location expression was:
>
> 0x00da: TAG_variable [9]
>  AT_location( fbreg -16 )
>  AT_name( "x" )
>  AT_decl_file(...)
>  AT_decl_line( 4 )
>  AT_type( {0x0204} ( $sxD ) )
>
> Note I removed the "deref" from the location expression. In this case the 
> variable would need to be read from memory. If the value was to be in a 
> register, then the location would be:
>
> 0x00da: TAG_variable [9]
>  AT_location( reg16 )
>  AT_name( "x" )
>  AT_decl_file(...)
>  AT_decl_line( 4 )
>  AT_type( {0x0204} ( $sxD ) )
>
> Then the value would be in register 16 itself (not dereferenced).
>
> Hope this all makes sense. If you have any questions let me know.
>

Thanks. I tend to agree this is the right thing to do, but it happens
to break many tests in the debugger.

Failing Tests (26):

lldb-Suite :: functionalities/unwind/sigtramp/TestSigtrampUnwind.py
lldb-Suite :: lang/c/blocks/TestBlocks.py
lldb-Suite :: lang/swift/address_of/TestSwiftAddressOf.py
lldb-Suite ::
lang/swift/expression/class_constrained_protocol/TestClassConstrainedProtocol.py
lldb-Suite ::
lang/swift/expression/exclusivity_suppression/TestExclusivitySuppression.py
lldb-Suite ::
lang/swift/expression/optional_amibiguity/TestOptionalAmbiguity.py
lldb-Suite :: lang/swift/expression/scopes/TestExpressionScopes.py
lldb-Suite :: lang/swift/expression/weak_self/TestWeakSelf.py
lldb-Suite ::
lang/swift/foundation_value_types/data/TestSwiftFoundationTypeData.py
lldb-Suite ::
lang/swift/foundation_value_types/date/TestSwiftFoundationTypeDate.py
lldb-Suite ::
lang/swift/foundation_value_types/indexpath/TestSwiftFoundationTypeIndexPath.py
lldb-Suite ::
lang/swift/foundation_value_types/measurement/TestSwiftFoundationTypeMeasurement.py
lldb-Suite ::
lang/swift/foundation_value_types/notification/TestSwiftFoundationTypeNotification.py
lldb-Suite ::
lang/swift/foundation_value_types/url/TestSwiftFoundationTypeURL.py
lldb-Suite ::
lang/swift/foundation_value_types/uuid/TestSwiftFoundationTypeUUID.py
lldb-Suite :: lang/swift/generic_tuple/TestSwiftGenericTuple.py
lldb-Suite :: lang/swift/po/sys_types/TestSwiftPOSysTypes.py

Re: [lldb-dev] DW_OP_deref handling

[Greg Clayton via lldb-dev]

I am happy to look at any DWARF expressions you have and help figure out where 
the change needs to go or how the expression need to be fixed.

Greg


> On Oct 11, 2018, at 12:32 PM, Davide Italiano  wrote:
> 
> On Thu, Oct 11, 2018 at 11:16 AM Greg Clayton  > wrote:
>> 
>> The issue is DW_OP_deref dereferences a pointer and pushes it onto the 
>> stack. The code currently, for a load address that is on top of the stack 
>> does:
>> 
>> Value::ValueType value_type = stack.back().GetValueType();
>> switch (value_type) {
>> case Value::eValueTypeLoadAddress:
>>  if (exe_ctx) {
>>if (process) {
>>  lldb::addr_t pointer_addr = 
>> stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
>>  Status error;
>>  lldb::addr_t pointer_value = 
>> process->ReadPointerFromMemory(pointer_addr, error);
>>  if (pointer_value != LLDB_INVALID_ADDRESS) {
>>stack.back().GetScalar() = pointer_value;
>>stack.back().ClearContext();
>>  } else {
>>return false;
>>  }
>> 
>> Here we just use "stack.back()" to access the item at the top of the 
>> expression value stack. The top value has a Value::ValueType of 
>> Value::eValueTypeLoadAddress. We don't reset this value to be 
>> Value::eValueTypeScalar, but we probably should and that will probably fix 
>> this issue.
>> 
>> DWARF5 finally added the ability to track what each value means on the 
>> expression stack. Prior to DWARF 5, we had no idea what each entry on the 
>> expression value stack was (file address, load address 
>> (Value::eValueTypeLoadAddress), plain value (Value::eValueTypeScalar). We 
>> have tracked this info for a while now, but the DWARF5 spec is much more 
>> specific on how things should be treated. From the spec:
>> 
>> DW_OP_deref
>> 
>> The DW_OP_deref operation pops the top stack entry and treats it as an 
>> address. The popped value must have an integral type. The value retrieved 
>> from that address is pushed, and has the generic type. The size of the data 
>> retrieved from the dereferenced address is the size of an address on the 
>> target machine.
>> 
>> No the "The value retrieved from that address is pushed, and has the generic 
>> type." statement. In LLDB this means the value's ValueType should be set to 
>> Value::eValueTypeScalar.
>> 
>> So try modifying the code with 
>> stack.back().SetValueType(Value::eValueTypeScalar) after the 
>> stack.back().ClearContext():
>> 
>>  if (pointer_value != LLDB_INVALID_ADDRESS) {
>>stack.back().GetScalar() = pointer_value;
>>stack.back().ClearContext();
>>stack.back().SetValueType(Value::eValueTypeScalar);
>>  } else {
>>return false;
>>  }
>> 
>> That might fix things.
>> 
>> The way the expression ends up right now we end up with an expression stack 
>> saying that the value is a load address which means we must read the value 
>> from memory in order to find it. This would be the right thing to do if your 
>> location expression was:
>> 
>> 0x00da: TAG_variable [9]
>> AT_location( fbreg -16 )
>> AT_name( "x" )
>> AT_decl_file(...)
>> AT_decl_line( 4 )
>> AT_type( {0x0204} ( $sxD ) )
>> 
>> Note I removed the "deref" from the location expression. In this case the 
>> variable would need to be read from memory. If the value was to be in a 
>> register, then the location would be:
>> 
>> 0x00da: TAG_variable [9]
>> AT_location( reg16 )
>> AT_name( "x" )
>> AT_decl_file(...)
>> AT_decl_line( 4 )
>> AT_type( {0x0204} ( $sxD ) )
>> 
>> Then the value would be in register 16 itself (not dereferenced).
>> 
>> Hope this all makes sense. If you have any questions let me know.
>> 
> 
> Thanks. I tend to agree this is the right thing to do, but it happens
> to break many tests in the debugger.
> 
> Failing Tests (26):
> 
>lldb-Suite :: functionalities/unwind/sigtramp/TestSigtrampUnwind.py
>lldb-Suite :: lang/c/blocks/TestBlocks.py
>lldb-Suite :: lang/swift/address_of/TestSwiftAddressOf.py
>lldb-Suite ::
> lang/swift/expression/class_constrained_protocol/TestClassConstrainedProtocol.py
>lldb-Suite ::
> lang/swift/expression/exclusivity_suppression/TestExclusivitySuppression.py
>lldb-Suite ::
> lang/swift/expression/optional_amibiguity/TestOptionalAmbiguity.py
>lldb-Suite :: lang/swift/expression/scopes/TestExpressionScopes.py
>lldb-Suite :: lang/swift/expression/weak_self/TestWeakSelf.py
>lldb-Suite ::
> lang/swift/foundation_value_types/data/TestSwiftFoundationTypeData.py
>lldb-Suite ::
> lang/swift/foundation_value_types/date/TestSwiftFoundationTypeDate.py
>lldb-Suite ::
> lang/swift/foundation_value_types/indexpath/TestSwiftFoundationTypeIndexPath.py
>lldb-Suite ::
> lang/swift/foundation_value_types/measurement/TestSwiftFoundationTypeMeasurement.py
>lldb-Suite ::
> lang/swift/foundation_value_types/n

Re: [lldb-dev] DW_OP_deref handling

[Davide Italiano via lldb-dev]

On Thu, Oct 11, 2018 at 1:38 PM Greg Clayton  wrote:
>
> I am happy to look at any DWARF expressions you have and help figure out 
> where the change needs to go or how the expression need to be fixed.
>

Thank you for your help, it's greatly appreciated. hopefully we'll
have a more precise debugger after this effort :)

--
Davide
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in case of missing location at block begin

[Matthias Braun via lldb-dev]

I reverted things in r344318 now.

> On Oct 10, 2018, at 5:02 PM, Jim Ingham  wrote:
> 
> Thanks for looking into this!  
> 
> When I was first working on inlined stepping, I found a bunch of cases where 
> the line table info and the ranges for the inlined subroutines disagreed.  I 
> remember seeing cases, for instance, where the address given for foo.h:xxx in 
> the line table was contained in one of the debug_info's inlined subroutine 
> blocks from a different file.  I tried for a little while to put in 
> heuristics to try to work past the disagreement.  But that was too easy to 
> get wrong, and when I got that wrong it often had the result of turning a 
> step into a continue.  It is annoying to stop too early, but it is much worse 
> to stop too late (or never).  So I ended up bagging my attempts at heroics 
> and whenever I get to a place where the line table and the inlined_subroutine 
> bits of info are out of sync, I just stop.
Makes total sense from lldbs point of view. If anything this is something a 
verifier could catch during testing, but it's a small issue so I'm not sure 
it's worth starting this now.

- Matthias

> 
> Jim
> 
> 
>> On Oct 10, 2018, at 4:34 PM, Matthias Braun  wrote:
>> 
>> 1) So I went and figured out why the lldb testcase at hand fails.
>> 
>> - It seems the debugger stepping logic will follow the program along until 
>> it finds a new source location. However if that new source location is part 
>> of a new DW_AT_abstract_location it is ignored in the step over mode.
>> - In the testcase at hand the .loc location of an inlined function was moved 
>> to an earlier place without the DW_AT_abstract_location entry getting 
>> extended. So the debugger mistook the inlined function as the same scope and 
>> stepping incorrectly halted there.
>> 
>> On the LLVM side DW_AT_abstract_location is generated by LexicalScopes which 
>> is created by lib/CodeGen/LexicalScopes.cpp / extractLexicalScopes() with 
>> completely separate code from the line table generation code in 
>> lib/CodeGen/AsmPrinter/DwarfDebug.cpp you have to be aware of that and keep 
>> the two algorithms in sync :-/
>> 
>> I fixed LexicalScopes.cpp to be in sync and the lldb test works fine again 
>> for me.
>> 
>> 2) However talking to Adrian earlier he also expressed having a bad feeling 
>> about moving debug location upwards instead of emitting line-0 entries. So I 
>> think consensus here is to rather live with some line table bloat instead.
>> 
>> - Unless there are objections I will not go with option 1) but instead 
>> revert this commit tomorrow. Note that I will only revert r343874 (i.e. the 
>> behavior change for what to emit when the first instructions of a basic 
>> block have no location attached), but not r343895 (removing debuglocs from 
>> spill/reload instructions) as I still consider this a sensible commit. So in 
>> the end we may have bigger line tables than before my changes, but simpler 
>> code in the spill/reload generation and occasionally can avoid random source 
>> location when spill/reloads get rescheduled.
>> 
>> - Matthias
>> 
>> 
>>> On Oct 10, 2018, at 1:17 PM, via llvm-commits  
>>> wrote:
>>> 
>>> 
>>> 
 -Original Message-
 From: Matthias Braun [mailto:ma...@braunis.de]
 Sent: Wednesday, October 10, 2018 3:50 PM
 To: Robinson, Paul
 Cc: jing...@apple.com; v...@apple.com; llvm-comm...@lists.llvm.org; lldb-
 d...@lists.llvm.org
 Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in
 case of missing location at block begin
 
 
 
> On Oct 10, 2018, at 12:18 PM, via llvm-commits >>> comm...@lists.llvm.org> wrote:
> 
> 
> 
>> -Original Message-
>> From: jing...@apple.com [mailto:jing...@apple.com]
>> Sent: Wednesday, October 10, 2018 2:20 PM
>> To: Vedant Kumar
>> Cc: Robinson, Paul; Vedant Kumar via llvm-commits; LLDB; Matthias Braun
>> Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location
 in
>> case of missing location at block begin
>> 
>> 
>> 
>>> On Oct 10, 2018, at 9:54 AM, Vedant Kumar  wrote:
>>> 
 On Oct 10, 2018, at 9:16 AM, Matthias Braun  wrote:
 
 So I haven't worked much on debug info, but here's the explanation
 for
>> my patches:
 My original motivation was getting rid of code some code in the llvm
>> codegen that for spills and reloads just picked the next debug location
>> around. That just seemed wrong to me, as spills and reloads really are
>> bookkeeping, we just move values around between registers and memory
 and
>> none of it is related to anything the user wrote in the source program.
 So
>> not assigning any debug information felt right for these instructions.
>> Then people noticed line table bloat because of this and I guess we
>> assumed that having exact line-0 annotations isn't that useful that it
>> warra

Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in case of missing location at block begin

[Adrian Prantl via lldb-dev]

> On Oct 11, 2018, at 4:42 PM, Matthias Braun  wrote:
> 
> I reverted things in r344318 now.
> 
>> On Oct 10, 2018, at 5:02 PM, Jim Ingham  wrote:
>> 
>> Thanks for looking into this!  
>> 
>> When I was first working on inlined stepping, I found a bunch of cases where 
>> the line table info and the ranges for the inlined subroutines disagreed.  I 
>> remember seeing cases, for instance, where the address given for foo.h:xxx 
>> in the line table was contained in one of the debug_info's inlined 
>> subroutine blocks from a different file.  I tried for a little while to put 
>> in heuristics to try to work past the disagreement.  But that was too easy 
>> to get wrong, and when I got that wrong it often had the result of turning a 
>> step into a continue.  It is annoying to stop too early, but it is much 
>> worse to stop too late (or never).  So I ended up bagging my attempts at 
>> heroics and whenever I get to a place where the line table and the 
>> inlined_subroutine bits of info are out of sync, I just stop.
> Makes total sense from lldbs point of view. If anything this is something a 
> verifier could catch during testing, but it's a small issue so I'm not sure 
> it's worth starting this now.

Thanks for getting to the bottom of this!

-- adrian
> 
> - Matthias
> 
>> 
>> Jim
>> 
>> 
>>> On Oct 10, 2018, at 4:34 PM, Matthias Braun  wrote:
>>> 
>>> 1) So I went and figured out why the lldb testcase at hand fails.
>>> 
>>> - It seems the debugger stepping logic will follow the program along until 
>>> it finds a new source location. However if that new source location is part 
>>> of a new DW_AT_abstract_location it is ignored in the step over mode.
>>> - In the testcase at hand the .loc location of an inlined function was 
>>> moved to an earlier place without the DW_AT_abstract_location entry getting 
>>> extended. So the debugger mistook the inlined function as the same scope 
>>> and stepping incorrectly halted there.
>>> 
>>> On the LLVM side DW_AT_abstract_location is generated by LexicalScopes 
>>> which is created by lib/CodeGen/LexicalScopes.cpp / extractLexicalScopes() 
>>> with completely separate code from the line table generation code in 
>>> lib/CodeGen/AsmPrinter/DwarfDebug.cpp you have to be aware of that and keep 
>>> the two algorithms in sync :-/
>>> 
>>> I fixed LexicalScopes.cpp to be in sync and the lldb test works fine again 
>>> for me.
>>> 
>>> 2) However talking to Adrian earlier he also expressed having a bad feeling 
>>> about moving debug location upwards instead of emitting line-0 entries. So 
>>> I think consensus here is to rather live with some line table bloat instead.
>>> 
>>> - Unless there are objections I will not go with option 1) but instead 
>>> revert this commit tomorrow. Note that I will only revert r343874 (i.e. the 
>>> behavior change for what to emit when the first instructions of a basic 
>>> block have no location attached), but not r343895 (removing debuglocs from 
>>> spill/reload instructions) as I still consider this a sensible commit. So 
>>> in the end we may have bigger line tables than before my changes, but 
>>> simpler code in the spill/reload generation and occasionally can avoid 
>>> random source location when spill/reloads get rescheduled.
>>> 
>>> - Matthias
>>> 
>>> 
 On Oct 10, 2018, at 1:17 PM, via llvm-commits 
  wrote:
 
 
 
> -Original Message-
> From: Matthias Braun [mailto:ma...@braunis.de]
> Sent: Wednesday, October 10, 2018 3:50 PM
> To: Robinson, Paul
> Cc: jing...@apple.com; v...@apple.com; llvm-comm...@lists.llvm.org; lldb-
> d...@lists.llvm.org
> Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in
> case of missing location at block begin
> 
> 
> 
>> On Oct 10, 2018, at 12:18 PM, via llvm-commits  comm...@lists.llvm.org> wrote:
>> 
>> 
>> 
>>> -Original Message-
>>> From: jing...@apple.com [mailto:jing...@apple.com]
>>> Sent: Wednesday, October 10, 2018 2:20 PM
>>> To: Vedant Kumar
>>> Cc: Robinson, Paul; Vedant Kumar via llvm-commits; LLDB; Matthias Braun
>>> Subject: Re: [lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location
> in
>>> case of missing location at block begin
>>> 
>>> 
>>> 
 On Oct 10, 2018, at 9:54 AM, Vedant Kumar  wrote:
 
> On Oct 10, 2018, at 9:16 AM, Matthias Braun  wrote:
> 
> So I haven't worked much on debug info, but here's the explanation
> for
>>> my patches:
> My original motivation was getting rid of code some code in the llvm
>>> codegen that for spills and reloads just picked the next debug location
>>> around. That just seemed wrong to me, as spills and reloads really are
>>> bookkeeping, we just move values around between registers and memory
> and
>>> none of it is related to anything the user wrote in the source program.
> So
>>> not