Bug 520952: Use filename when handling function breakpoints in console

Change-Id: I6bcdc658bf4c9453cdbe156808b292296a214fde

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsSynchronizer.java

@@ -24,6 +24,7 @@ import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.eclipse.cdt.core.IAddress;
@@ -764,8 +765,30 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI
 		}
 	}
 
+	/**
+	 * Create a new platform breakpoint for the function breakpoint. This method is
+	 * called when =breakpoint-created is received from GDB and there is not already
+	 * a matching platform breakpoint
+	 *
+	 * @param fileName
+	 *            resolved filename
+	 * @param miBpt
+	 *            breakpoint info from GDB, must be one for which
+	 *            {@link #isFunctionBreakpoint(MIBreakpoint)} returns true.
+	 * @return new platform breakpoint
+	 * @throws CoreException
+	 */
 	private ICBreakpoint createPlatformFunctionBreakpoint(String fileName, MIBreakpoint miBpt) throws CoreException {
-		IResource resource = getResource(fileName);
+		IResource resource;
+		String resolvedFileName;
+
+		if (userRequestedSpecificFile(miBpt)) {
+			resource = getResource(fileName);
+			resolvedFileName = fileName;
+		} else {
+			resource = ResourcesPlugin.getWorkspace().getRoot();
+			resolvedFileName = null;
+		}
 
 		int type = 0;
 		if (miBpt.isTemporary()) {
@@ -776,7 +799,7 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI
 		}
 		
 		return CDIDebugModel.createFunctionBreakpoint(
-				fileName, 
+				resolvedFileName,
 				resource, 
 				type, 
 				getFunctionName(miBpt), 
@@ -789,6 +812,23 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI
 				false);
 	}
 
+	/**
+	 * If the user inserted the breakpoint with a filename (e.g. "b main.c:main")
+	 * then create the breakpoint with that file, otherwise the function breakpoint
+	 * should be inserted in the same way as if it was done with the UI "Add
+	 * Function Breakpoint (C/C++)".
+	 * 
+	 * @param miBpt
+	 *            an MI Breakpoint that is a function breakpoint
+	 * @return true if the user specified file and function, false if just a
+	 *         function was specified.
+	 */
+	private boolean userRequestedSpecificFile(MIBreakpoint miBpt) {
+		assert isFunctionBreakpoint(miBpt);
+		String originalLocation = miBpt.getOriginalLocation();
+		return originalLocation != null && originalLocation.contains(":"); //$NON-NLS-1$
+	}
+
 	private ICBreakpoint createPlatformLineBreakpoint(String fileName, MIBreakpoint miBpt) throws CoreException {
 		IResource resource = getResource(fileName);
 
@@ -1152,7 +1192,7 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI
     	}
     	if (plBpt instanceof ICFunctionBreakpoint) {
     		return isFunctionBreakpoint(miBpt) ? 
-    			isPlatformFunctionBreakpoint((ICFunctionBreakpoint)plBpt, miBpt) : false;
+    			isPlatformFunctionBreakpoint((ICFunctionBreakpoint)plBpt, miBpt, fileName) : false;
     	}
     	try {
     		if (fileName == null || plBpt.getSourceHandle() == null 
@@ -1170,15 +1210,28 @@ public class MIBreakpointsSynchronizer extends AbstractDsfService implements IMI
     	return false;
     }
 
-    private boolean isPlatformFunctionBreakpoint(ICFunctionBreakpoint plBpt, MIBreakpoint miBpt) {
+	private boolean isPlatformFunctionBreakpoint(ICFunctionBreakpoint plBpt, MIBreakpoint miBpt, String fileName) {
 		try {
-			return (plBpt.getFunction() != null && plBpt.getFunction().equals(getFunctionName(miBpt)));
+			if (!Objects.equals(plBpt.getFunction(), getFunctionName(miBpt))) {
+				return false;
+			}
+			if (userRequestedSpecificFile(miBpt)) {
+				if (fileName == null || plBpt.getSourceHandle() == null
+						|| !new File(fileName).equals(new File(plBpt.getSourceHandle()))) {
+					return false;
+				}
+			} else {
+				if (plBpt.getSourceHandle() != null) {
+					return false;
+				}
+			}
+			return true;
 		}
 		catch(CoreException e) {
 			GdbPlugin.log(e.getStatus());
 		}
 		return false;
-    }
+	}
 
     private boolean isPlatformAddressBreakpoint(ICAddressBreakpoint plBpt, MIBreakpoint miBpt) {
 		try {

dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBConsoleBreakpointsTest.java

@@ -11,18 +11,26 @@
 
 package org.eclipse.cdt.tests.dsf.gdb.tests;
 
+import static org.junit.Assert.assertEquals;
+
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
+import org.eclipse.cdt.debug.core.CDIDebugModel;
 import org.eclipse.cdt.debug.core.model.ICAddressBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICFunctionBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICLineBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICWatchpoint;
 import org.eclipse.cdt.debug.internal.core.breakpoints.CBreakpoint;
+import org.eclipse.cdt.debug.internal.core.breakpoints.CFunctionBreakpoint;
 import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
 import org.eclipse.cdt.dsf.concurrent.Query;
 import org.eclipse.cdt.dsf.datamodel.DMContexts;
@@ -43,6 +51,7 @@ import org.eclipse.cdt.tests.dsf.gdb.framework.BaseParametrizedTestCase;
 import org.eclipse.cdt.tests.dsf.gdb.framework.SyncUtil;
 import org.eclipse.cdt.tests.dsf.gdb.launching.TestsPlugin;
 import org.eclipse.cdt.utils.Addr64;
+import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
@@ -165,6 +174,15 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
 				getLocationBreakpointAttributes(ICFunctionBreakpoint.class, true));
 	}
 
+	@Test
+	public void testValidFunctionNameOnlyBreakpoints() throws Throwable {
+		Map<String, Object> breakpointAttributes = getLocationBreakpointAttributes(ICFunctionBreakpoint.class, true);
+		breakpointAttributes.remove(ATTR_FILE_NAME);
+		testConsoleBreakpoint(
+				ICFunctionBreakpoint.class, 
+				breakpointAttributes);
+	}
+
 	@Test
 	public void testInvalidFunctionBreakpoints() throws Throwable {
 		testConsoleBreakpoint(
@@ -172,6 +190,15 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
 				getLocationBreakpointAttributes(ICFunctionBreakpoint.class, false));
 	}
 
+	@Test
+	public void testInvalidFunctionNameOnlyBreakpoints() throws Throwable {
+		Map<String, Object> breakpointAttributes = getLocationBreakpointAttributes(ICFunctionBreakpoint.class, false);
+		breakpointAttributes.remove(ATTR_FILE_NAME);
+		testConsoleBreakpoint(
+				ICFunctionBreakpoint.class, 
+				breakpointAttributes);
+	}
+
 	@Test
 	public void testValidAddressBreakpoints() throws Throwable {
 		testConsoleBreakpoint(
@@ -206,6 +233,220 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
 				ICWatchpoint.class, 
 				getWatchpointAttributes(ICWatchpoint.class, true, true));
 	}
+	
+	/**
+	 * Shortcut to CDIDebugModel.createFunctionBreakpoint
+	 */
+	private static void createFunctionBreakpoint(String filename, String function) throws CoreException {
+		CDIDebugModel.createFunctionBreakpoint(filename, ResourcesPlugin.getWorkspace().getRoot(), 0,
+				function, -1, -1, -1, true, 0, "", true);
+	}
+
+	private List<IBreakpoint> getPlatformBreakpoints(Predicate<IBreakpoint> predicate) {
+		return Arrays.asList(DebugPlugin.getDefault().getBreakpointManager().getBreakpoints()).stream()
+				.filter(predicate).collect(Collectors.toList());
+	}
+
+	private List<IBreakpoint> getPlatformFunctionBreakpoints() {
+		return getPlatformBreakpoints(CFunctionBreakpoint.class::isInstance);
+	}
+
+	/**
+	 * Test of the tests. This test ensures that basic creating/deleting of a function breakpoint works
+	 * as expected for the other  testFunctionBreakpointsAreIndependent* tests.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent0() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		setConsoleFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		assertEquals(1, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+	/**
+	 * Check that console inserted breakpoint with explicit file does not share platform
+	 * breakpoint that is not for a file.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent1() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(null, FUNCTION_VALID);
+		bps = getPlatformFunctionBreakpoints();
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(2, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(1, getTargetBreakpoints().length);
+
+		bps.get(1).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+	/**
+	 * Check that console inserted breakpoint without explicit file does not share platform
+	 * breakpoint that is for a file.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent2() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(null, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(2, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(1, getTargetBreakpoints().length);
+
+		bps.get(1).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+	/**
+	 * Check that console inserted breakpoint with explicit file does not share platform
+	 * breakpoint that is for a different file.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent3() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(SOURCE_NAME_INVALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(2, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(1, getTargetBreakpoints().length);
+
+		bps.get(1).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+	/**
+	 * Check that console inserted breakpoint without explicit file shares platform breakpoint
+	 * without file. This means that when the 1 platform breakpoint is deleted, both
+	 * target breakpoints should be removed.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent4() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(null, FUNCTION_VALID);
+		bps = getPlatformFunctionBreakpoints();
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(null, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+
+	/**
+	 * Check that console inserted breakpoint with explicit file shares platform breakpoint
+	 * with a file. This means that when the 1 platform breakpoint is deleted, both
+	 * target breakpoints should be removed.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent5() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(SOURCE_NAME_VALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+
+	/**
+	 * Check that console inserted breakpoint with explicit (invalid) file shares platform breakpoint
+	 * with (invalid) file. This means that when the 1 platform breakpoint is deleted, both
+	 * target breakpoints should be removed.
+	 */
+	@Test
+	public void testFunctionBreakpointsAreIndependent6() throws Throwable {
+		List<IBreakpoint> bps = getPlatformFunctionBreakpoints();
+		assertEquals(0, bps.size());
+
+		createFunctionBreakpoint(SOURCE_NAME_INVALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		setConsoleFunctionBreakpoint(SOURCE_NAME_INVALID, FUNCTION_VALID);
+		waitForBreakpointEvent(IBreakpointsAddedEvent.class);
+		bps = getPlatformFunctionBreakpoints();
+		assertEquals(1, bps.size());
+
+		assertEquals(2, getTargetBreakpoints().length);
+
+		bps.get(0).delete();
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		waitForBreakpointEvent(IBreakpointsRemovedEvent.class);
+		assertEquals(0, getTargetBreakpoints().length);
+	}
+	
 
 	@DsfServiceEventHandler
 	public void eventDispatched(IBreakpointsChangedEvent e) {
@@ -361,9 +602,13 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
   		queueConsoleCommand(String.format("break %s:%d", fileName, lineNumber));
   	}
 
-  	private void setConsoleFunctionBreakpoint(String fileName, String function) throws Throwable {
+	private void setConsoleFunctionBreakpoint(String fileName, String function) throws Throwable {
-  		queueConsoleCommand(String.format("break %s:%s", fileName, function));
+		if (fileName == null) {
-  	}
+			queueConsoleCommand(String.format("break %s", function));
+		} else {
+			queueConsoleCommand(String.format("break %s:%s", fileName, function));
+		}
+	}
 
   	private void setConsoleAddressBreakpoint(String address) throws Throwable {
   		queueConsoleCommand(String.format("break *%s", address));
@@ -407,24 +652,25 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
 		};
 		fSession.getExecutor().execute(query);
 		return query.get(timeout, unit).getMIBreakpoints();
-  	}
+	}
 
-  	private void waitForBreakpointEvent(Class<? extends IBreakpointsChangedEvent> eventType) throws Exception {
+	private void waitForBreakpointEvent(Class<? extends IBreakpointsChangedEvent> eventType) throws Exception {
-  		waitForBreakpointEvent(eventType, DEFAULT_TIMEOUT);
+		waitForBreakpointEvent(eventType, DEFAULT_TIMEOUT);
-  	}
+	}
 
-  	private void waitForBreakpointEvent(Class<? extends IBreakpointsChangedEvent> eventType, int timeout) throws Exception {
+	private void waitForBreakpointEvent(Class<? extends IBreakpointsChangedEvent> eventType, int timeout)
+			throws Exception {
+		long start = System.currentTimeMillis();
+		while (System.currentTimeMillis() <= start + timeout) {
+			if (breakpointEventReceived(eventType)) {
+				return;
+			}
+			synchronized (this) {
+				wait(timeout);
+			}
+		}
 		if (!breakpointEventReceived(eventType)) {
-			synchronized(this) {
+			throw new Exception(String.format("Timed out waiting for '%s' to occur.", eventType.getName()));
-				try {
-					wait(timeout);
-				} 
-				catch (InterruptedException ex) {
-				}
-			}
-			if (!breakpointEventReceived(eventType)) {
-				throw new Exception(String.format("Timed out waiting for '%s' to occur.", eventType.getName()));
-			}
 		}
 	}
 
@@ -461,7 +707,7 @@ public class GDBConsoleBreakpointsTest extends BaseParametrizedTestCase {
   	private ICFunctionBreakpoint findPlatformFunctionBreakpoint(String fileName, String function) throws Throwable {
   		for(IBreakpoint b : DebugPlugin.getDefault().getBreakpointManager().getBreakpoints()) {
   			if (b instanceof ICFunctionBreakpoint 
-  				&& fileName.equals(((ICLineBreakpoint)b).getSourceHandle()) 
+  				&& Objects.equals(fileName, ((ICLineBreakpoint)b).getSourceHandle())
   				&& function.equals(((ICLineBreakpoint)b).getFunction())) {
   				return (ICFunctionBreakpoint)b;
   			}