Skip to content

Commit

Permalink
fix: Breakpoints do not get triggered in Golang projects running C ex…
Browse files Browse the repository at this point in the history
…tensions via cgo (bazelbuild#6416)
  • Loading branch information
iliakondratev committed Oct 14, 2024
1 parent ac627ad commit 50ad8e8
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 11 deletions.
3 changes: 3 additions & 0 deletions aspect/intellij_info_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ def collect_go_info(target, ctx, semantics, ide_info, ide_info_file, output_grou
"""Updates Go-specific output groups, returns false if not a recognized Go target."""
sources = []
generated = []
cgo = False

# currently there's no Go Skylark API, with the only exception being proto_library targets
if ctx.rule.kind in [
Expand All @@ -434,6 +435,7 @@ def collect_go_info(target, ctx, semantics, ide_info, ide_info_file, output_grou
]:
sources = [f for src in getattr(ctx.rule.attr, "srcs", []) for f in src.files.to_list()]
generated = [f for f in sources if not f.is_source]
cgo = getattr(ctx.rule.attr, "cgo", False)
elif ctx.rule.kind == "go_wrap_cc":
genfiles = target.files.to_list()
go_genfiles = [f for f in genfiles if f.basename.endswith(".go")]
Expand Down Expand Up @@ -472,6 +474,7 @@ def collect_go_info(target, ctx, semantics, ide_info, ide_info_file, output_grou
import_path = import_path,
library_labels = library_labels,
sources = [artifact_location(f) for f in sources],
cgo = cgo,
)

compile_files = target[OutputGroupInfo].compilation_outputs if hasattr(target[OutputGroupInfo], "compilation_outputs") else depset([])
Expand Down
29 changes: 24 additions & 5 deletions base/src/com/google/idea/blaze/base/ideinfo/GoIdeInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ public final class GoIdeInfo implements ProtoWrapper<IntellijIdeInfo.GoIdeInfo>
private final ImmutableList<ArtifactLocation> sources;
@Nullable private final String importPath;
private final ImmutableList<Label> libraryLabels; // only valid for tests
private final boolean cgo;

private GoIdeInfo(
ImmutableList<ArtifactLocation> sources,
@Nullable String importPath,
ImmutableList<Label> libraryLabels) {
ImmutableList<Label> libraryLabels,
boolean cgo) {
this.sources = sources;
this.importPath = importPath;
this.libraryLabels = libraryLabels;
this.cgo = cgo;
}

public static GoIdeInfo fromProto(
Expand All @@ -48,7 +51,8 @@ public static GoIdeInfo fromProto(
ProtoWrapper.map(proto.getSourcesList(), ArtifactLocation::fromProto),
ImportPathReplacer.fixImportPath(
Strings.emptyToNull(proto.getImportPath()), targetLabel, targetKind),
extractLibraryLabels(targetKind, proto.getLibraryLabelsList()));
extractLibraryLabels(targetKind, proto.getLibraryLabelsList()),
proto.getCgo());
}

private static ImmutableList<Label> extractLibraryLabels(Kind kind, List<String> libraryLabels) {
Expand All @@ -66,6 +70,7 @@ public IntellijIdeInfo.GoIdeInfo toProto() {
IntellijIdeInfo.GoIdeInfo.newBuilder().addAllSources(ProtoWrapper.mapToProtos(sources));
ProtoWrapper.setIfNotNull(builder::setImportPath, importPath);
builder.addAllLibraryLabels(ProtoWrapper.map(libraryLabels, Label::toProto));
builder.setCgo(cgo);
return builder.build();
}

Expand All @@ -82,6 +87,10 @@ public ImmutableList<Label> getLibraryLabels() {
return libraryLabels;
}

public boolean getCgo() {
return cgo;
}

public static Builder builder() {
return new Builder();
}
Expand All @@ -91,6 +100,7 @@ public static class Builder {
private final ImmutableList.Builder<ArtifactLocation> sources = ImmutableList.builder();
@Nullable private String importPath = null;
private final ImmutableList.Builder<Label> libraryLabels = ImmutableList.builder();
private boolean cgo = false;

@CanIgnoreReturnValue
public Builder addSource(ArtifactLocation source) {
Expand All @@ -110,8 +120,14 @@ public Builder addLibraryLabel(String libraryLabel) {
return this;
}

@CanIgnoreReturnValue
public Builder setIsCgo(boolean cgo) {
this.cgo = cgo;
return this;
}

public GoIdeInfo build() {
return new GoIdeInfo(sources.build(), importPath, libraryLabels.build());
return new GoIdeInfo(sources.build(), importPath, libraryLabels.build(), cgo);
}
}

Expand All @@ -128,6 +144,8 @@ public String toString() {
+ " libraryLabels="
+ getLibraryLabels()
+ "\n"
+ " cgo="
+ getCgo()
+ '}';
}

Expand All @@ -142,11 +160,12 @@ public boolean equals(Object o) {
GoIdeInfo goIdeInfo = (GoIdeInfo) o;
return Objects.equals(sources, goIdeInfo.sources)
&& Objects.equals(importPath, goIdeInfo.importPath)
&& Objects.equals(libraryLabels, goIdeInfo.libraryLabels);
&& Objects.equals(libraryLabels, goIdeInfo.libraryLabels)
&& cgo == goIdeInfo.cgo;
}

@Override
public int hashCode() {
return Objects.hash(sources, importPath, libraryLabels);
return Objects.hash(sources, importPath, libraryLabels, cgo);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ public BlazeProjectData build() {
blazeInfo =
BlazeInfo.createMockBlazeInfo(
outputBase,
outputBase + "/execroot",
outputBase + "/execroot/bin",
outputBase + "/execroot/gen",
outputBase + "/execroot/testlogs");
outputBase + "/execroot/_main",
outputBase + "/execroot/_main/bin",
outputBase + "/execroot/_main/gen",
outputBase + "/execroot/_main/testlogs");
}
BlazeVersionData blazeVersionData =
this.blazeVersionData != null ? this.blazeVersionData : BlazeVersionData.builder().build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,35 @@
import com.goide.dlv.location.DlvPositionConverter;
import com.goide.dlv.location.DlvPositionConverterFactory;
import com.goide.sdk.GoSdkService;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.Maps;
import com.google.idea.blaze.base.ideinfo.ArtifactLocation;
import com.google.idea.blaze.base.ideinfo.GoIdeInfo;
import com.google.idea.blaze.base.ideinfo.TargetIdeInfo;
import com.google.idea.blaze.base.ideinfo.TargetMap;
import com.google.idea.blaze.base.io.VfsUtils;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.model.primitives.ExecutionRootPath;
import com.google.idea.blaze.base.model.primitives.WorkspaceRoot;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.sync.workspace.ExecutionRootPathResolver;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.ui.MessageType;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.xdebugger.impl.XDebuggerManagerImpl;
import org.jetbrains.annotations.NotNull;

import java.io.File;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.annotation.Nullable;

import static java.util.Collections.emptySet;
import static java.util.stream.Collectors.toSet;

class BlazeDlvPositionConverter implements DlvPositionConverter {
private static final Logger logger = Logger.getInstance(BlazeDlvPositionConverter.class);

Expand All @@ -39,17 +55,21 @@ class BlazeDlvPositionConverter implements DlvPositionConverter {
private final ExecutionRootPathResolver resolver;
private final Map<VirtualFile, String> localToRemote;
private final Map<String, VirtualFile> normalizedToLocal;
private final CgoTrimmedPathsHandler cgoTrimmedPathsHandler;

private BlazeDlvPositionConverter(
WorkspaceRoot workspaceRoot,
String goRoot,
ExecutionRootPathResolver resolver,
Set<String> remotePaths) {
Set<String> remotePaths,
CgoTrimmedPathsHandler cgoTrimmedPathsHandler) {
this.root = workspaceRoot;
this.goRoot = goRoot;
this.resolver = resolver;
this.localToRemote = Maps.newHashMapWithExpectedSize(remotePaths.size());
this.normalizedToLocal = Maps.newHashMapWithExpectedSize(remotePaths.size());
this.cgoTrimmedPathsHandler = cgoTrimmedPathsHandler;

for (String path : remotePaths) {
String normalized = normalizePath(path);
if (normalizedToLocal.containsKey(normalized)) {
Expand Down Expand Up @@ -113,6 +133,8 @@ private String normalizePath(String path) {
return afterNthSlash(path, 4);
} else if (path.startsWith("GOROOT/")) {
return goRoot + '/' + afterNthSlash(path, 1);
} else if (cgoTrimmedPathsHandler.matchesCgoTrimmedPath(path)) {
return cgoTrimmedPathsHandler.normalizeCgoTrimmedPath(path);
}
return path;
}
Expand Down Expand Up @@ -141,8 +163,93 @@ public DlvPositionConverter createPositionConverter(
String goRoot = GoSdkService.getInstance(project).getSdk(module).getHomePath();
ExecutionRootPathResolver resolver = ExecutionRootPathResolver.fromProject(project);
return (workspaceRoot != null && resolver != null)
? new BlazeDlvPositionConverter(workspaceRoot, goRoot, resolver, remotePaths)
? new BlazeDlvPositionConverter(workspaceRoot, goRoot, resolver, remotePaths, new CgoTrimmedPathsHandler(project, resolver))
: null;
}
}

/**
* This class is responsible for identifying and normalizing paths that may have been trimmed by cgo—a tool in Go for integrating C code.
*
* <p>Potential issues addressed:
* <ul>
* <li>Uncertainty about whether sources were trimmed by cgo (the `cgo=true` flag in a Bazel target doesn't guarantee all source paths are trimmed; it depends on whether Go files import C code).</li>
* <li>Ambiguities from name collisions between workspace names and file names.</li>
* </ul>
*
* <p>Key functionalities:
* <ul>
* <li>Identify if a path matches a cgo-trimmed path based on Bazel workspace name.</li>
* <li>Normalize cgo-trimmed paths while handling potential collisions.</li>
* </ul>
*
* <p>In a rare occasion when multiple source files are detected for the same path, a warning will be shown to user.
*/

static class CgoTrimmedPathsHandler {
private final Project project;
private final Set<String> cgoSources;
private final Set<String> nonCgoSources;
private final String bazelWorkspaceRelativePath;

public CgoTrimmedPathsHandler(Project project, ExecutionRootPathResolver resolver) {
this.project = project;
BlazeProjectData projectData = BlazeProjectDataManager.getInstance(project).getBlazeProjectData();
boolean hasCgoTargets = projectData != null && projectData.getTargetMap().targets().stream()
.map(TargetIdeInfo::getGoIdeInfo)
.filter(Objects::nonNull)
.anyMatch(GoIdeInfo::getCgo);

this.cgoSources = hasCgoTargets ? collectCgoSources(projectData.getTargetMap()) : emptySet();
this.nonCgoSources = hasCgoTargets ? collectNonCgoSources(projectData.getTargetMap()) : emptySet();
this.bazelWorkspaceRelativePath = resolver.getExecutionRoot().getName() + File.separator;
}

public boolean matchesCgoTrimmedPath(String path) {
return !cgoSources.isEmpty() && path.startsWith(this.bazelWorkspaceRelativePath);
}

public String normalizeCgoTrimmedPath(String path) {
String normalizedPath = afterNthSlash(path, 1);
if (cgoSources.contains(normalizedPath)) {
if (!nonCgoSources.contains(path) && !cgoSources.contains(path)) {
return normalizedPath;
} else {
XDebuggerManagerImpl.getNotificationGroup()
.createNotification(
"Multiple source files detected for the same path: " + path + ".\n" +
"For these source files, breakpoints may not function correctly.\n" +
"Check for possible collisions between Bazel workspace names and Go package names.",
MessageType.WARNING)
.notify(project);
return nonCgoSources.contains(path) ? path : normalizedPath;
}
} else {
return path;
}
}
}

private static @NotNull Set<String> collectCgoSources(TargetMap targetMap) {
return targetMap.targets().stream()
.map(TargetIdeInfo::getGoIdeInfo)
.filter(Objects::nonNull)
.filter(GoIdeInfo::getCgo)
.map(GoIdeInfo::getSources)
.flatMap(ImmutableCollection::stream)
.filter(ArtifactLocation::isMainWorkspaceSourceArtifact)
.map(ArtifactLocation::getExecutionRootRelativePath)
.collect(toSet());
}

private static @NotNull Set<String> collectNonCgoSources(TargetMap targetMap) {
return targetMap.targets().stream()
.map(TargetIdeInfo::getGoIdeInfo)
.filter(Objects::nonNull)
.filter(goIdeInfo -> !goIdeInfo.getCgo())
.map(GoIdeInfo::getSources)
.flatMap(ImmutableCollection::stream)
.map(ArtifactLocation::getExecutionRootRelativePath)
.collect(toSet());
}
}
Loading

0 comments on commit 50ad8e8

Please sign in to comment.