Community
Participate
Working Groups
Build Identifier: I20100608-0911 The function 'org.eclipse.cdt.debug.core.code.cdi.model.ICDIVariable#getValue()' builds and returns a new value if the current value is 'null'. If the the current variable is of type ICDIArrayType the method calls 'getHexAddress()': value = new Array(this, getHexAddress()); // Variable.java, Line 366 The problem is that getHexAddress() calls getValue() again, which creates an infinite loop: hexAddress = v.getValue().getValueString(); // Variable.java, Line 168 So it is wrong to call 'getHexAddress()' from 'getValue()' if 'value' is 'null'. This line: value = new Array(this, getHexAddress()); // Variable.java, Line 366 should be: value = new Array(this, null); // Variable.java, Line 366 Reproducible: Always Steps to Reproduce: Happens every time with the Intel debugger when trying to expand XMM registers in the register window.
I think it would help to see a stacktrace of the infinite loop / recursion.
(In reply to comment #1) > I think it would help to see a stacktrace of the infinite loop / recursion. Sure, please see below the stack and mi traces. Big registers, e.g., XMM registers, are represented as arrays. The address of the arrays that are stored in registers is thus undefined, but Eclipse keeps asking for an address in a recursive fashion. Regards, Nicolas ============================================== Stack trace: Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 Register(Variable).getHexAddress() line: 166 Register(Variable).getValue() line: 364 CLocalVariable$InternalVariable.getValue() line: 213 CLocalVariable(CVariable).getValue() line: 251 CVariableLabelProvider.getLabel(TreePath, IPresentationContext, String) line: 46 CVariableLabelProvider(ElementLabelProvider).getLabel(TreePath, IPresentationContext, String, int) line: 312 CVariableLabelProvider(ElementLabelProvider).retrieveLabel(ILabelUpdate) line: 215 ElementLabelProvider$LabelUpdater.run() line: 160 ElementLabelProvider$LabelJob.run(IProgressMonitor) line: 74 Worker.run() line: 54 Here is the mi trace: [1,283,419,238,628] 164^done,lang="C++",expr="$xmm0" [1,283,419,238,629] (gdb) [1,283,419,247,416] 165-var-create - * &(($xmm0).v16_int8) [1,283,419,247,424] 165^done,name="var53",numchild="16",value="", type="short:8 [16]" [1,283,419,247,425] 166-var-set-format var53 hexadecimal [1,283,419,247,425] (gdb) [1,283,419,247,430] 166^done,format="hexadecimal", value="" [1,283,419,247,431] (gdb) [1,283,419,252,815] 167-var-set-format var53 hexadecimal [1,283,419,252,818] 167^done,format="hexadecimal", value="" [1,283,419,252,819] (gdb) [1,283,419,254,336] 168-var-set-format var53 hexadecimal [1,283,419,254,340] 168^done,format="hexadecimal", value="" [1,283,419,254,341] (gdb) [1,283,419,255,135] 169-var-set-format var53 hexadecimal [1,283,419,255,140] 169^done,format="hexadecimal", value="" [1,283,419,255,141] (gdb) [1,283,419,255,814] 170-var-set-format var53 hexadecimal [1,283,419,255,818] 170^done,format="hexadecimal", value="" [1,283,419,255,819] (gdb) [1,283,419,256,647] 171-var-set-format var53 hexadecimal [1,283,419,256,650] 171^done,format="hexadecimal", value="" [1,283,419,256,651] (gdb) [1,283,419,257,351] 172-var-set-format var53 hexadecimal [1,283,419,257,354] 172^done,format="hexadecimal", value="" [1,283,419,257,355] (gdb) [1,283,419,259,095] 173-var-set-format var53 hexadecimal [1,283,419,259,098] 173^done,format="hexadecimal", value="" [1,283,419,259,101] (gdb) [1,283,419,259,699] 174-var-set-format var53 hexadecimal [1,283,419,259,701] 174^done,format="hexadecimal", value="" [1,283,419,259,703] (gdb) [1,283,419,260,213] 175-var-set-format var53 hexadecimal [1,283,419,260,215] 175^done,format="hexadecimal", value="" [1,283,419,260,218] (gdb) [1,283,419,260,788] 176-var-set-format var53 hexadecimal [1,283,419,260,791] 176^done,format="hexadecimal", value="" [1,283,419,260,793] (gdb) [1,283,419,261,181] 177-var-set-format var53 hexadecimal [1,283,419,261,185] 177^done,format="hexadecimal", value="" [1,283,419,261,186] (gdb)
I am not very familiar with this code, so I wonder if it can make sense to call getHexAddress() for other cases, i.e. we might break something if we just replace the call with 'null'? Have you tried with the DSF-GDB debugger yet? CDI is considered unsupported since 7.0.
(In reply to comment #3) > I am not very familiar with this code, so I wonder if it can make sense to call > getHexAddress() for other cases, i.e. we might break something if we just > replace the call with 'null'? > Have you tried with the DSF-GDB debugger yet? CDI is considered unsupported > since 7.0. So instead of value = new ArrayValue(this, getHexAddress()); What about: value = new ArrayValue(this, hexAddress); This would stop the recursion if the address is undefined without much impact on other cases. This bug does only occur in CDI mode but we need to keep supporting it. Thx, Nicolas
(In reply to comment #4) > So instead of > > value = new ArrayValue(this, getHexAddress()); > > What about: > > value = new ArrayValue(this, hexAddress); > > This would stop the recursion if the address is undefined without > much impact on other cases. This bug does only occur in CDI mode but > we need to keep supporting it. This would be basically the same as new ArrayValue(this, null), because hexAddress is only initialized upon the first call to getHexAddress() and getHexAddress() is only called from getValue(), therefore it will always be null. I'll attach a patch with my suggested solution from comment 3.
Created attachment 178125 [details] Suggested fix Could you try this?
(In reply to comment #6) > Created an attachment (id=178125) [details] > Suggested fix > Could you try this? Your patch fixes this issue and is more general. I am playing with the debugger and everything looks right. Thanks for your prompt answer. Nicolas
Excellent. I tested with GDB and everything seems to be normal. Do you need a fix in 7.0.1?
(In reply to comment #8) > Excellent. I tested with GDB and everything seems to be normal. > Do you need a fix in 7.0.1? That would be perfect.
John, would you review my changes before I commit?
(In reply to comment #6) > Created an attachment (id=178125) [details] > Suggested fix > Actually a memory mapped register can have an address
Created attachment 178131 [details] Alternative fix Alternative fix. Hexadecimal address is needed only later when the array pointer value is needed. Postpone this.
Created attachment 178132 [details] patch for getting correctly value of a register child Also, recursion is due to find mi register algorithm that does not take in account that a register variable can have children. Thus the search should check for the qualified name as well. i.e. "$mm4" should be a different variable object then "&($mm4)"
(In reply to comment #11) > (In reply to comment #6) > > Created an attachment (id=178125) [details] [details] > > Suggested fix > > > Actually a memory mapped register can have an address Shouldn't a memory mapped register be treated as a volatile variable?
(In reply to comment #12) > Created an attachment (id=178131) [details] > Alternative fix > Alternative fix. Hexadecimal address is needed only later when the array > pointer value is needed. Postpone this. This solution works for me too.
(In reply to comment #13) > Created an attachment (id=178132) [details] > patch for getting correctly value of a register child > > Also, recursion is due to find mi register algorithm that does not take in > account that a register variable can have children. Thus the search should > check for the qualified name as well. > > i.e. "$mm4" should be a different variable object then "&($mm4)" Looks like this patch alone should already take care of the recursion? I suggest to use this patch for 7.0.1. The "Alternative fix" adds new API which is not allowed on a maintenance branch. I would also add a check in getHexAddress() to test for an immediate recursion: // make sure to avoid infinite recursion if (v != this) { v.setFormat(ICDIFormat.HEXADECIMAL); hexAddress = v.getValue().getValueString(); } else { hexAddress = ""; }
Created attachment 178143 [details] patch for getting value of register child (updated) Incorporate Toni's feedback
(In reply to comment #16) > Looks like this patch alone should already take care of the recursion? > I suggest to use this patch for 7.0.1. > The "Alternative fix" adds new API which is not allowed on a maintenance > branch. > Agree. Though I would still commit on head. To lessen the cost of creating ArrayValue objects. > I would also add a check in getHexAddress() to test for an immediate recursion: > Done in the updated patch. Nicolas, could you please check if applying only register child value patch works on Intel debugger as well? Thank you, Teo
(In reply to comment #18) > (In reply to comment #16) > > Looks like this patch alone should already take care of the recursion? > > I suggest to use this patch for 7.0.1. > > The "Alternative fix" adds new API which is not allowed on a maintenance > > branch. > > > Agree. Though I would still commit on head. To lessen the cost of creating > ArrayValue objects. > > I would also add a check in getHexAddress() to test for an immediate recursion: > > > Done in the updated patch. > Nicolas, could you please check if applying only register child value patch > works on Intel debugger as well? > Thank you, > Teo Hmm, the expressions I see are much more complex and recursion is still there. I need to check if it's not a problem on my side. So I'll post an update on Monday.
(In reply to comment #19) > > Hmm, the expressions I see are much more complex and recursion is still there. > I need to check if it's not a problem on my side. So I'll post an update on > Monday. The difference most likely are due to register children type how they are reported, and being interpreted as array for Intel debugger with intel debugger: 165-var-create - * &(($xmm0).v16_int8) 165^done,name="var53",numchild="16",value="",type="short:8[16]" with mingw gdb debugger (gdb 7.1): 1860-var-create - * &(($xmm0).v16_int8) 1860^done,name="var101",numchild="1",value="",type="int8_t (*)[16]" "int8_t (*)[16]" is interpreted as pointer to 16 element array of int8_t, while "short:8[16]" most probably is interpreted as 16 element array of "short:8" I am not very familiar with gdb type parser, but the call to org.eclipse.cdt.debug.mi.core.cdi.SourceManager.getType(Target, String) would be the starting point.
(In reply to comment #17) > Created an attachment (id=178143) [details] > patch for getting value of register child (updated) > > Incorporate Toni's feedback The patch contains a suspicious diff: @@ -353,8 +358,8 @@ } else if (t instanceof ICDIFloatType) { value = new FloatValue(this); } else if (t instanceof ICDIDoubleType) { - value = new DoubleValue(this); } else if (t instanceof ICDIFunctionType) { + value = new DoubleValue(this); value = new FunctionValue(this); } else if (t instanceof ICDIPointerType) { value = new PointerValue(this); I don't think you intended that change.
Created attachment 178256 [details] patch for getting value of register child (update2) remove unintended change
(In reply to comment #22) > Created an attachment (id=178256) [details] > patch for getting value of register child (update2) > remove unintended change My eclipse workspace is kind of mess up. I am getting a unresolved compilation problem when lauching eclipse in debug mode: java.lang.Error: Unresolved compilation problem:
(In reply to comment #23) (In reply to comment #22) > Created an attachment (id=178256) [details] [details] > patch for getting value of register child (update2) > remove unintended change My eclipse workspace is kind of mess up. I am getting a unresolved compilation problem when lauching eclipse in debug mode: java.lang.Error: Unresolved compilation problem: "!MESSAGE An internal error occurred during: "Initiating resource change handling..". !STACK 0 java.lang.Error: Unresolved compilation problem: The method addSaveParticipant(Plugin, ISaveParticipant) in the type IWorkspace is not applicable for the arguments (String, ResourceChangeHandler) at org.eclipse.cdt.internal.core.settings.model.CProjectDescriptionManager.startSaveParticipant(CProjectDescriptionManager.java:362) at org.eclipse.cdt.internal.core.settings.model.CProjectDescriptionManager.access$1(CProjectDescriptionManager.java:359) at org.eclipse.cdt.internal.core.settings.model.CProjectDescriptionManager$3.run(CProjectDescriptionManager.java:334) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)" I am using the CDT revision from HEAD with eclipse 3.6.0. Everything looks fine when I compile the CDT sources. Did anyone experience a similar issue?
(In reply to comment #24) > I am using the CDT revision from HEAD with eclipse 3.6.0. Everything looks fine > when I compile the CDT sources. Did anyone experience a similar issue? This works for me in my HEAD workspace. Maybe you need to do a clean build and/or check the runtime plug-ins in your launch configuration.
To make some progress, I am going to commit my suggest change from comment 16 to cdt_7_0 (without any other changes). This is a safe change and should address the infinite recursion originally reported.
*** cdt cvs genie on behalf of aleherbau *** Bug 323630 - Expanding registers in the register view triggers an infinite recursion loop in Eclipse/CDI [*] Variable.java 1.37.2.1 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/model/Variable.java?root=Tools_Project&r1=1.37&r2=1.37.2.1
(In reply to comment #27) > *** cdt cvs genie on behalf of aleherbau *** > Bug 323630 - Expanding registers in the register view triggers an infinite > recursion loop in Eclipse/CDI > [*] Variable.java 1.37.2.1 > http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/model/Variable.java?root=Tools_Project&r1=1.37&r2=1.37.2.1 The bug does not show up anymore :-). Thanks, Nicolas
Thanks Nicolas. Teo, I committed your patches to HEAD.
*** cdt cvs genie on behalf of aleherbau *** Bug 323630 - Expanding registers in the register view triggers an infinite recursion loop in Eclipse/CDI [*] RegisterManager.java 1.24 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-debug/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/RegisterManager.java?root=Tools_Project&r1=1.23&r2=1.24 [*] ArrayValue.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-debug/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/model/type/ArrayValue.java?root=Tools_Project&r1=1.15&r2=1.16 [*] Variable.java 1.38 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-debug/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/model/Variable.java?root=Tools_Project&r1=1.37&r2=1.38 [*] MANIFEST.MF 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-debug/org.eclipse.cdt.debug.mi.core/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.18&r2=1.19 [*] feature.xml 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.gnu.debug-feature/feature.xml?root=Tools_Project&r1=1.20&r2=1.21 [*] buildsite.xml 1.40 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.releng/buildsite.xml?root=Tools_Project&r1=1.39&r2=1.40
