Bugzilla – Full Text Bug Listing
| Summary: | Variables view has problems with duplicate debug info for local variable | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Jens Elmenthaler <jens.elmenthaler> | ||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | Marc Khouzam <marc.khouzam> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | cdtdoug, pawel.1.piech | ||||||||
| Version: | 7.0 | ||||||||||
| Target Milestone: | 7.0.2 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
Created attachment 181616 [details]
screenshot
You're right, there is a such a fundamental assumption. IMO, the best way to handle it is with the suggested workaround. (In reply to comment #2) > You're right, there is a such a fundamental assumption. IMO, the best way to > handle it is with the suggested workaround. I suggested two options for a workaround. One is to fix it in DSF (this would mean around VariableVMNode, but here is possibly the fundamental assumption you are talking about), the other in DSF GDB (around MIStack). Which you do you refer to? I was talking about point 2: dropping the duplicate variable from getLocals(). There is a fundamental assumption that expressions created by IExpressions.createExpressions() are unique. I'm not sure if it would be worth it to try to break that assumption for this case. Do you agree? Created attachment 181807 [details]
Fix removing duplicates in MIStack
This patch eliminates the flickering of selection and as such brings it to parity with CDI. As such, it ensures the fundamental assumption in DSF for the expressions service about unique local variable names within the same stack frame is not violated.
However, gdb often picks the wrong location from the two alternatives provided by gcc when processing -var-XXX or -data-evaluate-expression, and thus might show wrong values. But CDI (and other gdb frontends) do so, too.
(In reply to comment #5) Thank you James, I'll let mark look at the patch when he gets back from vacation. I was just looking at the same issue in Bug 327621 :-) In that case, the duplicate names are both valid because one variable shadows another: int main() { int myvar=8; for (int myvar=0;myvar<5;myvar++) { printf("Here gdb reports two variables myVar since they are both in scope"); } } The flickering is bad and must be fixed. But in the case above, we are not dealing with a bug and I'm still thinking about how we can handle it properly. Funny thing is when I tried to fix it, I wrote a patch removing the duplicate from VariableVMNode instead of MIStack :-) (In reply to comment #7) > I was just looking at the same issue in Bug 327621 :-) > In that case, the duplicate names are both valid because one variable shadows > another: > int main() { > int myvar=8; > for (int myvar=0;myvar<5;myvar++) { > printf("Here gdb reports two variables myVar since they are both in > scope"); > } > } > The flickering is bad and must be fixed. But in the case above, we are not > dealing with a bug and I'm still thinking about how we can handle it properly. > Funny thing is when I tried to fix it, I wrote a patch removing the duplicate > from VariableVMNode instead of MIStack :-) Damn, damn, damn. See my comments in Bug 327621. (In reply to comment #8) > Damn, damn, damn. See my comments in Bug 327621. Ok, so since Bug 327621 is a superset of this bug, let's continue the discussion there, and once we have it fixed, we should be able to mark this bug fixed. Created attachment 183022 [details] Slightly modified solution I slightly modified Jens patch to simplify makeVariableDMCs(). Also, I think we don't need to keep track of the fact that a context hasDuplicates for two reasons; 1- the first instance given by GDB is the inner-most one, so the value is ok (note that we we don't actually use the value returned by -stack-list-locals anywhere, but I think it is safe if we did) 2- in the case of errors from gcc, we will send the second -stack-list-locals without asking for values. Jens, what do you think if this modification? I suggest we commit this first fix and continue on to bug 327621 (In reply to comment #10) > Created an attachment (id=183022) [details] > Slightly modified solution > I slightly modified Jens patch to simplify makeVariableDMCs(). Also, I think > we don't need to keep track of the fact that a context hasDuplicates for two > reasons; > 1- the first instance given by GDB is the inner-most one, so the value is ok > (note that we we don't actually use the value returned by -stack-list-locals > anywhere, but I think it is safe if we did) > 2- in the case of errors from gcc, we will send the second -stack-list-locals > without asking for values. It possibly doesn't really matter: when I run into this gcc bug, gdb returns two local variables with the same name at different locations, and thus having different values. There is no error returned by gdb, though. So we would pick the first value, which might be correct, or it might not. In order to get the maximum out of it, we could keep the value if all duplicates return the same value, and discard it otherwise? > Jens, what do you think if this modification? > I suggest we commit this first fix and continue on to bug 327621 Yes, that helps. I just read your post to the gdb mailing list. Your modification is OK. There is possibly nothing more we can get out of it. (In reply to comment #11) > (In reply to comment #10) > > Created an attachment (id=183022) [details] [details] > > Slightly modified solution > > I slightly modified Jens patch to simplify makeVariableDMCs(). Also, I think > > we don't need to keep track of the fact that a context hasDuplicates for two > > reasons; > > 1- the first instance given by GDB is the inner-most one, so the value is ok > > (note that we we don't actually use the value returned by -stack-list-locals > > anywhere, but I think it is safe if we did) > > 2- in the case of errors from gcc, we will send the second -stack-list-locals > > without asking for values. > It possibly doesn't really matter: when I run into this gcc bug, gdb returns > two local variables with the same name at different locations, and thus having > different values. There is no error returned by gdb, though. So we would pick > the first value, which might be correct, or it might not. In my case, I get an error from GDB. > In order to get the maximum out of it, we could keep the value if all > duplicates return the same value, and discard it otherwise? Yeah, that would be fine. I think it may be overkill at this point, but if you want to submit a patch it will be ok with me. (In reply to comment #12) > I just read your post to the gdb mailing list. Your modification is OK. There > is possibly nothing more we can get out of it. I committed the last patch to HEAD. Thanks Jens for the contribution. *** cdt cvs genie on behalf of mkhouzam *** Bug 328573: Variables view has problems with duplicate debug info for local variable [*] MIStack.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIStack.java?root=Tools_Project&r1=1.17&r2=1.18 Committed to 7_0 *** cdt cvs genie on behalf of mkhouzam *** Bug 328573: Variables view has problems with duplicate debug info for local variable [*] MIStack.java 1.16.2.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIStack.java?root=Tools_Project&r1=1.16.2.1&r2=1.16.2.2 |
Build Identifier: HEAD Create the following sources files: // Sequence.hpp class Sequence { public: Sequence(); virtual ~Sequence(); void setElementAt(int index); }; // Sequence.cpp #include "Sequence.hpp" Sequence::Sequence() { } Sequence::~Sequence() { } void Sequence::setElementAt(int index) { } // debug_info.cpp #include "Sequence.hpp" Sequence getSequence() { Sequence data; data.setElementAt(0); return data; } int main() { Sequence data = getSequence(); return 0; } Reproducible: Always Steps to Reproduce: 1. Compile the code 2. Set a break point into getSequence, and debug the program. When halting at the break point, you see two entries for the local variable data. Select one and the variables view is jumping back and forth between the two entries. Whether you can reproduce the problem or not, might actually depend ont he gcc version that is used for compiling. I used 4.1.2. Looking into the debug info (objdump -W debug_info.o) yields two entries for data: <1><9e>: Abbrev Number: 3 (DW_TAG_subprogram) DW_AT_sibling : <fb> DW_AT_external : 1 DW_AT_name : getSequence DW_AT_decl_file : 1 DW_AT_decl_line : 3 DW_AT_MIPS_linkage_name: _Z11getSequencev DW_AT_type : <93> DW_AT_low_pc : 0 DW_AT_high_pc : 0x4f DW_AT_frame_base : 0 (location list) <2><db>: Abbrev Number: 4 (DW_TAG_variable) DW_AT_name : data DW_AT_decl_file : 1 DW_AT_decl_line : 5 DW_AT_type : <93> DW_AT_accessibility: 3 (private) DW_AT_location : 2 byte block: 76 58 (DW_OP_breg6: -40) <2><eb>: Abbrev Number: 5 (DW_TAG_variable) DW_AT_name : data DW_AT_decl_file : 1 DW_AT_decl_line : 5 DW_AT_type : <93> DW_AT_location : 2 byte block: 91 50 (DW_OP_fbreg: -48) Because of this, gdb reponds in -stack-list-locals with two local variables called "data" which sub-sequently create problems. There might be a fundamental assumption in DSF that for each stack frame the name of local variables is unique. Even if it is fixed in later gcc version, I don't have the option to switch to another one. I found other complaining in the gcc mailing list complaining about this, but I didn't find a matching gcc or gdb bugzilla. I see two options to workaround this problems: 1) Fix in DSF, such that it can handle two local variables with the same name. I tested it wich CDI and it can handle it. I shows the same contain for both entries. 2) Workaround in the MIStack. If two variables with the same name are returned by -stack-list-locals discard one of them, including the values. You would have to discard the values, since both have different values and you don't know which one is the correct for the local variable. The expression service later doesn't seem to have a problem.