Bug 469842 - Breakpoint : disable relocation
Summary: Breakpoint : disable relocation
Status: RESOLVED FIXED
Alias: None
Product: TCF
Classification: Tools
Component: Agent (show other bugs)
Version: 1.3   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 1.4   Edit
Assignee: Project Inbox CLA
QA Contact: Eugene Tarassov CLA
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2015-06-10 09:42 EDT by xavier pouyollon CLA
Modified: 2015-06-17 02:31 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description xavier pouyollon CLA 2015-06-10 09:42:01 EDT
Hi Eugene,

Currently if a user tries to put a breakpoint on a comment line /* .... */, the breakpoint gets "adjusted" (adjustement occurs if no exact line match is done with the .debug_line).

Is there a way to disable this ? 

We have a customer that is using higly optimized code and this relocation sometimes end-ups putting breakpoints at "unwanted" location.

I'd like we can provide a property like "StrictBp" that would disable the relocation on Breakpoint add.

Thanks !
Xavier.
Comment 1 Martin Oberhuber CLA 2015-06-10 10:19:15 EDT
+1 this approach makes a lot of sense to me !

Let me see if I understand right:

Requesting a BP to be planted at a location, due to .debug_line information, one or more addresses are determined. Now each of the addresses "map back" to one or more locations which may or may not match the original location. The request is making it more configurable what to do with such "inexact matches":

- Today, it seems like all addresses are planted, whether exact or not
- Xavier's request is only planting exact matches and ignoring inexact ones
  --> May lead to the breakpoint not planted at all, how to visualize that ?

If the feature ends up being configurable as Xavier suggests, then what about allowing more fine granular configuration:
(a) Plant all locations, 
(b) Plant only if in the same file,
(c) Plant only if in same file and less than n lines away,
(d) Plant only exact matches.

Seems that today, TCF-Agent always does (a) and Xavier requests optionally (d).
Am I getting this right ? Eugene what are your thoughts ?
Comment 2 Eugene Tarassov CLA 2015-06-12 17:02:13 EDT
I have added new breakpoint property: LineOffset. When set, the breakpoint is not planted if "map back" offset is more than specified number of lines. It is sufficient to handle all cases (a) to (d):

(a) Plant all locations - the property is not set 
(b) Plant only if in the same file - any large number, e.g. 1000000
(c) Plant only if in same file and less than n lines away - set to n
(d) Plant only exact matches - set to 0

I also added GUI for the property in the Window/Preferences dialog.

Fixed.
Thanks!
Comment 3 xavier pouyollon CLA 2015-06-15 09:33:54 EDT
Great ! Many thanks !
Xavier.
Comment 4 Martin Oberhuber CLA 2015-06-15 09:47:31 EDT
That's great indeed !

Now I'd love to see some sample code that would help testing/demonstrating the new feature. I suspect that whole-program optimization together with "optimize for size" and "common subexpression elimination" might be one use case:
http://www.tldp.org/LDP/LGNET/issue71/joshi.html#sect4

Dead Code Elimination, together with a foreign file's WPO expanded inline right after the dead code might also help:
http://www.tldp.org/LDP/LGNET/issue71/joshi.html#sect5

Could anyone come up with a good use-case to check all the new options ?
Comment 5 xavier pouyollon CLA 2015-06-16 10:22:47 EDT
Hi Eugene,

Some reflexion about Breakpoint Relocation and the potential side issues.

Let's suppose a .c file with 3 functions a, b,c.

Line 10 a
Line 20 b
Line 30 c

The .debug_line might just contain (suppose an optimizing compiler).
@x to line 15
@y to line 22
@z to line 34

The .debug_line are not wrong per se but incomplete.

The user has breakpoint relocation enabled, plants a breakpoint at line 23 so in function b and it...ends up in function c !

Another possible issue:
We can also have cases where a ‘.h’ file is included by two (or more) ‘.c’ files, and if one wants to plant a breakpoint in the ‘.h’ file (e.g: because he wants to debug an inlined routine), we can end up in planting breakpoint in other function, because one compile unit does not instantiate the inlined routine, but another does.

So in one compile unit the breakpoint will be correctly planted, but on another, the breakpoint will be planted elsewhere, and probably in another function, due to the relocation feature. The user will wonder why such a breakpoint has been planted.

IMHO, breakpoint relocation is pretty dangerous feature. We should find a way to protect more or be more descriptive to the end-user.

2 suggestions:
- The linenumber service could return all possible lines where a bp can be planted so the IDE could "grey-out" the others.

- When planting if there is a valid match (understand planted on the exact line), don't try to do breakpoint relocation for other cases : it would solve the .h problem described above.

What's your opinion on this ?
Thanks !
Xavier.
Comment 6 Eugene Tarassov CLA 2015-06-16 15:07:41 EDT
> IMHO, breakpoint relocation is pretty dangerous feature.

There is no such feature. It does not exist. At least, not in the public TCF code.

What exists is incomplete line info produced by compiler, which causes line breakpoints to be planted imprecisely. The right way to fix this is to fix the compiler. In the debugger, we can workaround this by rejecting line info which is too imprecise, but it does not really help much. I'm not sure what else we could do.
Comment 7 xavier pouyollon CLA 2015-06-17 02:31:52 EDT
Hi Eugene,

> The right way to fix this is to fix the compiler.

I fully agree but we have to deal with existing released version of compiler that generate incomplete line numbers. We had complains from customer especially because breakpoint can be put in a "unexpected" functions (the next one). 

> In the debugger, we can workaround this by rejecting line info which is too imprecise, but it does not really help much. 

LineOffset set to 0 really helps because it avoids a breakpoint being put in the next function, which is really annoying. 

I sugested a service to return all the possible "valid" lines in a compile unit so the IDE can clearly display where breakpoint can be set for better understanding.

> I'm not sure what else we could do.

The breakpoint set in .h file (inlined function for instace. Ok that's a marginal case) can lead to breakpoint being set at unexpected places. That's why I suggested that if an exact match can be found, debugger should not try to plant "imprecisely" (what I called relocate) if the breakpoint has more than one @.

Thanks !