diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF b/dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF index e5bad1ccc36..699c56f85bd 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-Vendor: %providerName Bundle-SymbolicName: org.eclipse.cdt.dsf.gdb;singleton:=true -Bundle-Version: 4.0.0.qualifier +Bundle-Version: 4.0.1.qualifier Bundle-Activator: org.eclipse.cdt.dsf.gdb.internal.GdbPlugin Bundle-Localization: plugin Require-Bundle: org.eclipse.core.runtime, diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/pom.xml b/dsf-gdb/org.eclipse.cdt.dsf.gdb/pom.xml index f9139527dfd..2fc57ff2a9a 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/pom.xml +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/pom.xml @@ -11,7 +11,7 @@ ../../pom.xml - 4.0.0-SNAPSHOT + 4.0.1-SNAPSHOT org.eclipse.cdt.dsf.gdb eclipse-plugin diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java index 0e0e2655d2f..0f2e0180037 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIVariableManager.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 Monta Vista and others. + * Copyright (c) 2008, 2011 Monta Vista and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -12,6 +12,7 @@ * Ericsson - Major re-factoring to deal with children * Axel Mueller - Bug 306555 - Add support for cast to type / view as array (IExpressions2) * Jens Elmenthaler (Verigy) - Added Full GDB pretty-printing support (bug 302121) + * Axel Mueller - Workaround for GDB bug where -var-info-path-expression gives invalid result (Bug 320277) *******************************************************************************/ package org.eclipse.cdt.dsf.mi.service; @@ -64,6 +65,7 @@ import org.eclipse.cdt.dsf.mi.service.command.output.ExprMetaGetChildCountInfo; import org.eclipse.cdt.dsf.mi.service.command.output.ExprMetaGetChildrenInfo; import org.eclipse.cdt.dsf.mi.service.command.output.ExprMetaGetValueInfo; import org.eclipse.cdt.dsf.mi.service.command.output.ExprMetaGetVarInfo; +import org.eclipse.cdt.dsf.mi.service.command.output.MIDataEvaluateExpressionInfo; import org.eclipse.cdt.dsf.mi.service.command.output.MIDisplayHint; import org.eclipse.cdt.dsf.mi.service.command.output.MIDisplayHint.GdbDisplayHint; import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo; @@ -378,6 +380,15 @@ public class MIVariableManager implements ICommandControl { private boolean fetchingChildren = false; + + /** + * In case of base class variables that are accessed in a derived class + * we cannot trust var-info-path-expression because of a bug in gdb. + * We have to use a workaround and apply it to the complete hierarchy of this varObject. + * Bug 320277 + */ + private boolean hasCastToBaseClassWorkaround = false; + public MIVariableObject(VariableObjectId id, MIVariableObject parentObj) { this(id, parentObj, false); } @@ -1352,15 +1363,43 @@ public class MIVariableManager implements ICommandControl { numSubRequests++; - final DataRequestMonitor childPathRm = - new DataRequestMonitor(fSession.getExecutor(), countingRm) { + // Class to keep track of the child's full expression, but also + // if that child had to use the CastToBaseClassWorkaround, + // which needs to be propagated to its own children. + class ChildFullExpressionInfo { + private String childFullExpression; + private boolean childHasCastToBaseClassWorkaround; + + public ChildFullExpressionInfo(String path) { + this(path, false); + } + + public ChildFullExpressionInfo(String path, boolean hasWorkaround) { + childFullExpression = path == null ? "" : path; //$NON-NLS-1$ + childHasCastToBaseClassWorkaround = hasWorkaround; + } + + public String getChildPath() { return childFullExpression; } + public boolean getChildHasCastToBaseClassWorkaround() { return childHasCastToBaseClassWorkaround; } + }; + + final DataRequestMonitor childPathRm = + new DataRequestMonitor(fSession.getExecutor(), countingRm) { @Override protected void handleSuccess() { + final String childPath = getData().getChildPath(); + // The child varObj we are about to create should have hasCastToBaseClassWorkaround + // set in two conditions: + // 1- if its parent was set (which is the current varObj) + // 2- if the workaround was used for the child itself, which is part of ChildFullExpressionInfo + final boolean childHasCastToBaseClassWorkaround = + hasCastToBaseClassWorkaround || getData().getChildHasCastToBaseClassWorkaround(); + // For children that do not map to a real expression (such as f.public) // GDB returns an empty string. In this case, we can use another unique // name, such as the variable name - final boolean fakeChild = (getData().length() == 0); - final String childFullExpression = fakeChild ? child.getVarName() : getData(); + final boolean fakeChild = (childPath.length() == 0); + final String childFullExpression = fakeChild ? child.getVarName() : childPath; // Now try to see if we already have this variable object in our Map // Since our map names use the expression, and not the GDB given @@ -1395,6 +1434,8 @@ public class MIVariableManager implements ICommandControl { var = createChild(childId, childFullExpression, indexInParent, child); } + var.hasCastToBaseClassWorkaround = childHasCastToBaseClassWorkaround; + if (fakeChild) { addRealChildrenOfFake(var, exprDmc, realChildren, @@ -1425,6 +1466,8 @@ public class MIVariableManager implements ICommandControl { if (childVar == null) { childVar = createChild(childId, childFullExpression, indexInParent, child); + childVar.hasCastToBaseClassWorkaround = childHasCastToBaseClassWorkaround; + if (fakeChild) { addRealChildrenOfFake(childVar, exprDmc, realChildren, @@ -1442,14 +1485,21 @@ public class MIVariableManager implements ICommandControl { if (isAccessQualifier(child.getExp())) { // This is just a qualifier level of C++, so we don't need // to call -var-info-path-expression for real, but just pretend we did. - childPathRm.setData(""); //$NON-NLS-1$ + childPathRm.setData(new ChildFullExpressionInfo("")); //$NON-NLS-1$ childPathRm.done(); } else if (isDynamic() || exprInfo.hasDynamicAncestor()) { // Equivalent to (which can't be implemented): child.hasDynamicAncestor // The new child has a dynamic ancestor. Such children don't support // var-info-path-expression. Build the expression ourselves. - childPathRm.setData(buildChildExpression(exprDmc.getExpression(), child.getExp())); + childPathRm.setData(new ChildFullExpressionInfo(buildChildExpression(exprDmc.getExpression(), child.getExp()))); childPathRm.done(); + } else if (hasCastToBaseClassWorkaround) { + // We had to use the "CastToBaseClass" workaround in the hierarchy, so we + // know -var-info-path-expression won't work in this case. We have to + // build the expression ourselves again to keep the workaround as part + // of the child's expression. + childPathRm.setData(new ChildFullExpressionInfo(buildChildExpression(exprDmc.getExpression(), child.getExp()))); + childPathRm.done(); } else { // To build the child id, we need the fully qualified expression which we // can get from -var-info-path-expression starting from GDB 6.7 @@ -1459,14 +1509,55 @@ public class MIVariableManager implements ICommandControl { @Override protected void handleCompleted() { if (isSuccess()) { - childPathRm.setData(getData().getFullExpression()); + final String expression = getData().getFullExpression(); + + if (needFixForGDBBug320277() && child.getExp().equals(child.getType()) && !isAccessQualifier(getExpressionInfo().getRelExpr())) { + // Special handling for a derived class that is cast to its base class (see bug 320277) + // + // If the name of a child equals its type then it could be a base class. + // The exception is when the name of the actual variable is identical with the type name (bad coding style :-)) + // The only way to tell the difference is to check if the parent is a fake (public/private/protected). + // + // What we could do instead, is make sure we are using C++ (using -var-info-expression). That would + // be safer. However, at this time, there does not seem to be a way to create a variable with the same + // name and type using plain C, so we are safe. + // + // When we know we are dealing with derived class that is cast to its base class + // -var-info-path-expression returns (*(testbase*) this) and in some cases + // this expression will fail when being evaluated in GDB because of a GDB bug. + // Instead, we need (*(struct testbase*) this). + // + // To check if GDB actually has this bug we call -data-evaluate-expression with the return value + // of -var-info-path-expression + IExpressionDMContext exprDmcMIData = fExpressionService.createExpression(exprDmc, expression); + fCommandControl.queueCommand( + fCommandFactory.createMIDataEvaluateExpression(exprDmcMIData), + new DataRequestMonitor(fSession.getExecutor(), childPathRm) { + @Override + protected void handleCompleted() { + if (isSuccess()) { + childPathRm.setData(new ChildFullExpressionInfo(expression)); + childPathRm.done(); + } else { + // We build the expression ourselves + // We must also indicate that this workaround has been used for this child + // so that we know to keep using it for further descendants. + childPathRm.setData(new ChildFullExpressionInfo(buildDerivedChildExpression(exprDmc.getExpression(), child.getExp()), true)); + childPathRm.done(); + } + } + }); + } else { + childPathRm.setData(new ChildFullExpressionInfo(expression)); + childPathRm.done(); + } } else { // If we don't have var-info-path-expression // build the expression ourselves // Note that this does not work well yet - childPathRm.setData(buildChildExpression(exprDmc.getExpression(), child.getExp())); + childPathRm.setData(new ChildFullExpressionInfo(buildChildExpression(exprDmc.getExpression(), child.getExp()))); + childPathRm.done(); } - childPathRm.done(); } }); } @@ -1582,27 +1673,65 @@ public class MIVariableManager implements ICommandControl { }); } + /** + * Method performing special handling for a derived class that is cast to its base class (see bug 320277). + * The command '-var-info-path-expression' returns (*(testbase*) this) but we need (*(struct testbase*) this). + * Also, in case of a namespace this method adds additional backticks: + * (*(struct 'namespace::testbase'*) this) + */ + private String buildDerivedChildExpression(String parentExp, String childExpr) { + + final String CAST_PREFIX = "struct "; //$NON-NLS-1$ + + // Before doing the cast, let's surround the child expression (base class name) with quotes + // if it contains a :: which indicates a namespace + String childNameForCast = childExpr.contains("::") ? "'" + childExpr + "'" : childExpr; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + String childFullExpression; + if (isPointer()) { + // casting to pointer base class requires a slightly different format + childFullExpression = "*(" + CAST_PREFIX + childNameForCast + "*)(" + parentExp + ")";//$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + } else { + // casting to base class + childFullExpression = "(" + CAST_PREFIX + childNameForCast + ")" + parentExp;//$NON-NLS-1$ //$NON-NLS-2$ + } + + return childFullExpression; + } + /** * This method builds a child expression based on its parent's expression. * It is a fallback solution for when GDB doesn't support the var-info-path-expression. * - * Currently, this does not support inherited class such as + * This method does not take care of inherited classes such as * class foo : bar { * ... * } - * because we'll create foo.bar instead of (bar)foo. + * that case is hanlded by buildDerivedChildExpression */ private String buildChildExpression(String parentExp, String childExp) { + String childFullExpression; + + // If the current varObj is a fake object, we obtain the proper parent + // expression from the parent of the varObj. + if (isAccessQualifier(exprInfo.getRelExpr())) { + parentExp = getParent().getExpression(); + } + // For pointers, the child expression is already contained in the parent, // so we must simply prefix with * // See Bug219179 for more information. if (!isDynamic() && !exprInfo.hasDynamicAncestor() && isPointer()) { - return "*("+parentExp+")"; //$NON-NLS-1$//$NON-NLS-2$ + childFullExpression = "*("+parentExp+")"; //$NON-NLS-1$//$NON-NLS-2$ + } else { + // We must surround the parentExp with parentheses because it + // may be a casted expression. + childFullExpression = "("+parentExp+")." + childExp; //$NON-NLS-1$ //$NON-NLS-2$ } - - return parentExp + "." + childExp; //$NON-NLS-1$ + // No need for a special case for arrays since we deal with arrays differently // and don't call this method for them + + return childFullExpression; } /** @@ -2894,4 +3023,22 @@ public class MIVariableManager implements ICommandControl { iterator.remove(); } } + + /** + * GDB has a bug which makes -data-evaluate-expression fail when using + * the return value of -var-info-path-expression in the case of derived classes. + * To work around this bug, we don't use -var-info-path-expression for some derived + * classes and all their descendants. + * + * This method had to be kept private for the maintenance release. + * + * See http://sourceware.org/bugzilla/show_bug.cgi?id=11912 + * and Bug 320277. + * + * The bug was fixed in GDB 7.3.1. + * + */ + private boolean needFixForGDBBug320277() { + return true; + } } diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java index f196d3dbbdd..6686606254e 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIExpressionsTest.java @@ -61,7 +61,7 @@ public class MIExpressionsTest extends BaseTestCase { private DsfServicesTracker fServicesTracker; - private IExpressions fExpService; + protected IExpressions fExpService; private int fExprChangedEventCount = 0; diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_3/MIExpressionsTest_7_3.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_3/MIExpressionsTest_7_3.java index 6da31bc02bf..2b39678630a 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_3/MIExpressionsTest_7_3.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_3/MIExpressionsTest_7_3.java @@ -7,13 +7,29 @@ * * Contributors: * Ericsson - Initial Implementation + * Marc Khouzam (Ericsson) - Modify testDeleteChildren() for GDB output + * change (Bug 320277) *******************************************************************************/ package org.eclipse.cdt.tests.dsf.gdb.tests.tests_7_3; +import static org.junit.Assert.assertTrue; + +import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor; +import org.eclipse.cdt.dsf.debug.service.IExpressions.IExpressionDMContext; +import org.eclipse.cdt.dsf.debug.service.IExpressions.IExpressionDMData; +import org.eclipse.cdt.dsf.debug.service.IRunControl.StepType; +import org.eclipse.cdt.dsf.debug.service.IStack.IFrameDMContext; +import org.eclipse.cdt.dsf.mi.service.command.events.MIStoppedEvent; +import org.eclipse.cdt.tests.dsf.gdb.framework.AsyncCompletionWaitor; import org.eclipse.cdt.tests.dsf.gdb.framework.BackgroundRunner; +import org.eclipse.cdt.tests.dsf.gdb.framework.SyncUtil; +import org.eclipse.cdt.tests.dsf.gdb.launching.TestsPlugin; import org.eclipse.cdt.tests.dsf.gdb.tests.ITestConstants; import org.eclipse.cdt.tests.dsf.gdb.tests.MIExpressionsTest; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; import org.junit.BeforeClass; +import org.junit.Test; import org.junit.runner.RunWith; @RunWith(BackgroundRunner.class) @@ -22,4 +38,132 @@ public class MIExpressionsTest_7_3 extends MIExpressionsTest { public static void beforeClassMethod_7_3() { setGdbProgramNamesLaunchAttributes(ITestConstants.SUFFIX_GDB_7_3); } + + // Slight change in GDB output to fix a bug, so we must change the test a little + // Bug 320277 + @Override + @Test + public void testDeleteChildren() throws Throwable { + SyncUtil.runToLocation("testDeleteChildren"); + MIStoppedEvent stoppedEvent = SyncUtil.step(1, StepType.STEP_OVER); + final IFrameDMContext frameDmc = SyncUtil.getStackFrame(stoppedEvent.getDMContext(), 0); + + final AsyncCompletionWaitor wait = new AsyncCompletionWaitor(); + + fExpService.getExecutor().submit(new Runnable() { + public void run() { + + // First create the var object and all its children + IExpressionDMContext parentDmc = fExpService.createExpression(frameDmc, "f"); + + fExpService.getSubExpressions( + parentDmc, + new DataRequestMonitor(fExpService.getExecutor(), null) { + @Override + protected void handleCompleted() { + if (!isSuccess()) { + wait.waitFinished(getStatus()); + } else { + if (getData().length != 5) { + wait.waitFinished(new Status(IStatus.ERROR, TestsPlugin.PLUGIN_ID, + "Failed getting children; expecting 5 got " + getData().length, null)); + } else { + String childStr = "((class bar) f)"; + if (!getData()[0].getExpression().equals(childStr)) { + wait.waitFinished(new Status(IStatus.ERROR, TestsPlugin.PLUGIN_ID, + "Got child " + getData()[0].getExpression() + " instead of " + childStr, null)); + } else { + // Now list the children of the first element + fExpService.getSubExpressions( + getData()[0], + new DataRequestMonitor(fExpService.getExecutor(), null) { + @Override + protected void handleCompleted() { + if (!isSuccess()) { + wait.waitFinished(getStatus()); + } else { + if (getData().length != 2) { + wait.waitFinished(new Status(IStatus.ERROR, TestsPlugin.PLUGIN_ID, + "Failed getting children; expecting 2 got " + getData().length, null)); + } else { + String childStr = "((((class bar) f)).d)"; + if (!getData()[0].getExpression().equals(childStr)) { + wait.waitFinished(new Status(IStatus.ERROR, TestsPlugin.PLUGIN_ID, + "Got child " + getData()[0].getExpression() + " instead of " + childStr, null)); + } else { + wait.setReturnInfo(getData()[0]); + wait.waitFinished(); + } + } + } + } + }); + } + } + } + } + }); + } + }); + + wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER); + assertTrue(wait.getMessage(), wait.isOK()); + final IExpressionDMContext deletedChildDmc = (IExpressionDMContext)wait.getReturnInfo(); + + wait.waitReset(); + + fExpService.getExecutor().submit(new Runnable() { + public void run() { + + // Now create more than 1000 expressions to trigger the deletion of the children + // that were created above + for (int i=0; i<1100; i++) { + IExpressionDMContext dmc = fExpService.createExpression(frameDmc, "a[" + i + "]"); + + wait.increment(); + fExpService.getExpressionData( + dmc, + new DataRequestMonitor(fExpService.getExecutor(), null) { + @Override + protected void handleCompleted() { + if (!isSuccess()) { + wait.waitFinished(getStatus()); + } else { + wait.waitFinished(); + } + } + }); + } + } + }); + + wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER); + assertTrue(wait.getMessage(), wait.isOK()); + wait.waitReset(); + + fExpService.getExecutor().submit(new Runnable() { + public void run() { + + // Evaluate the expression of a child that we know is deleted to make sure + // the expression service can handle that + fExpService.getExpressionData( + deletedChildDmc, + new DataRequestMonitor(fExpService.getExecutor(), null) { + @Override + protected void handleCompleted() { + if (!isSuccess()) { + wait.waitFinished(getStatus()); + } else { + wait.waitFinished(); + } + } + }); + } + }); + + wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER); + assertTrue(wait.getMessage(), wait.isOK()); + wait.waitReset(); + + } }