lttng.ust: Add the build-ID to the key of cache of addr2line calls

Before calling addr2line, we should also verify that the buildId
we got from the trace matches the file on disk we are about to
look at.

This buildId is already present in the UstDebugInfoBinaryFile
objects returned by the analysis, it is just a matter of
passing it down to the FileOffsetMapper call.

This way we can use it as part of the key for the cache of
calls, so that eventually calling it on different binaries
yields separate calls.

The actual verification is not done at the moment, as this
would require calling a separate process (like "eu-readelf").
There is not much gain in doing it yet anyway, because we only
look for one possible file (the one at the expected path). If
eventually the analysis is extended to look through several
different files for a given path - using separate debug symbol
files for examples - then the surrounding code won't have to
be modified.

Change-Id: I11f42a4295c88f66ad4a685198e30181daf0a1ba
Signed-off-by: Alexandre Montplaisir <alexmonthy@efficios.com>
Reviewed-on: https://git.eclipse.org/r/70306
Reviewed-by: Hudson CI
Reviewed-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Tested-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>

diff --git a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/debuginfo/FileOffsetMapper.java b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/debuginfo/FileOffsetMapper.java
index 40c5598d618d9f4f6cb858643d136c2d3430a4a3..4a83cbb1217fd94e3cad47a7cffdcca5ff95729a 100644 (file)
--- a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/debuginfo/FileOffsetMapper.java
+++ b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/debuginfo/FileOffsetMapper.java
@@ -50,16 +50,18 @@ public final class FileOffsetMapper {
     private static class FileOffset {
 
         private final String fFilePath;
+        private final String fBuildId;
         private final long fOffset;
 
-        public FileOffset(String filePath, long offset) {
+        public FileOffset(String filePath, String buildId, long offset) {
             fFilePath = filePath;
+            fBuildId = buildId;
             fOffset = offset;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hashCode(fFilePath, fOffset);
+            return Objects.hashCode(fFilePath, fBuildId, fOffset);
         }
 
         @Override
@@ -74,13 +76,9 @@ public final class FileOffsetMapper {
                 return false;
             }
             FileOffset other = (FileOffset) obj;
-            if (!fFilePath.equals(other.fFilePath)) {
-                return false;
-            }
-            if (fOffset != other.fOffset) {
-                return false;
-            }
-            return true;
+            return Objects.equal(fFilePath, other.fFilePath) &&
+                    Objects.equal(fBuildId, other.fBuildId) &&
+                    Objects.equal(fOffset, other.fOffset);
         }
     }
 
@@ -112,16 +110,22 @@ public final class FileOffsetMapper {
      *
      * @param file
      *            The binary file to look at
+     * @param buildId
+     *            The expected buildId of the binary file (is not verified at
+     *            the moment)
      * @param offset
      *            The memory offset in the file
      * @return The list of callsites corresponding to the offset, reported from
      *         the "highest" inlining location, down to the initial definition.
      */
-    public static @Nullable Iterable<TmfCallsite> getCallsiteFromOffset(File file, long offset) {
+    public static @Nullable Iterable<TmfCallsite> getCallsiteFromOffset(File file, String buildId, long offset) {
         if (!Files.exists((file.toPath()))) {
             return null;
         }
-        FileOffset fo = new FileOffset(checkNotNull(file.toString()), offset);
+        // TODO We should also eventually verify that the passed buildId matches
+        // the file we are attempting to open.
+
+        FileOffset fo = new FileOffset(checkNotNull(file.toString()), buildId, offset);
         return CALLSITE_CACHE.getUnchecked(fo);
     }
 

diff --git a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/BinaryCallsite.java b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/BinaryCallsite.java
index 26791f3ca6e3d41b580884194c14844f03bc4d09..1c55e4cc4b9bdbe446939272aaaed6e1d07948e7 100644 (file)
--- a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/BinaryCallsite.java
+++ b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/BinaryCallsite.java
@@ -25,6 +25,7 @@ package org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo;
 public class BinaryCallsite {
 
     private final String fBinaryFilePath;
+    private final String fBuildId;
     private final String fSymbolName;
     private final long fOffset;
 
@@ -34,6 +35,10 @@ public class BinaryCallsite {
      * @param binaryFilePath
      *            The path to the binary file on disk, as specified in the trace
      *            (may or may not be present on the system opening the trace).
+     * @param buildId
+     *            The Build-Id of the binary file. This is an unique identifier
+     *            for a given object, so it can be used to make sure the file at
+     *            the given path is the exact one we expect.
      * @param symbolName
      *            The name of the symbol in the path. Should not be null, but
      *            can be an empty string if not available.
@@ -41,12 +46,13 @@ public class BinaryCallsite {
      *            The offset *within the binary* of the call site. This should
      *            be ready to be passed as-is to tools like addr2line.
      */
-    public BinaryCallsite(String binaryFilePath, String symbolName, long offset) {
+    public BinaryCallsite(String binaryFilePath, String buildId, String symbolName, long offset) {
         if (offset < 0) {
             throw new IllegalArgumentException("Address offset cannot be negative"); //$NON-NLS-1$
         }
 
         fBinaryFilePath = binaryFilePath;
+        fBuildId = buildId;
         fSymbolName = symbolName;
         fOffset = offset;
     }
@@ -60,6 +66,15 @@ public class BinaryCallsite {
         return fBinaryFilePath;
     }
 
+    /**
+     * The build-id of the binary
+     *
+     * @return The build-id
+     */
+    public String getBuildId() {
+        return fBuildId;
+    }
+
     /**
      * Get the name of the symbol this instruction pointer is from, if it is
      * available.

diff --git a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoBinaryAspect.java b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoBinaryAspect.java
index 4bb345e2dd3310f283c8f4ced7a3e24695057350..942b7678ad71c372aa30f82fecaeb9dfbd493ace 100644 (file)
--- a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoBinaryAspect.java
+++ b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoBinaryAspect.java
@@ -122,7 +122,7 @@ public class UstDebugInfoBinaryAspect implements ITmfEventAspect<BinaryCallsite>
         // TODO If the binary is present on the current file system, we could
         // try to get the symbol name from it.
 
-        return new BinaryCallsite(fullPath, EMPTY_STRING, offset);
+        return new BinaryCallsite(fullPath, file.getBuildId(), EMPTY_STRING, offset);
     }
 
     /**

diff --git a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java
index fbeed9c4760e844f0e3928fb4159d5109466dfb7..03d1352e3a8be29d9e06887c994a6b8461d0b294 100644 (file)
--- a/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java
+++ b/lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java
@@ -78,7 +78,10 @@ public class UstDebugInfoSourceAspect implements ITmfEventAspect<TmfCallsite> {
      *         and line number
      */
     public static @Nullable TmfCallsite getSourceCallsite(LttngUstTrace trace, BinaryCallsite bc) {
-        Iterable<TmfCallsite> callsites = FileOffsetMapper.getCallsiteFromOffset(new File(bc.getBinaryFilePath()), bc.getOffset());
+        Iterable<TmfCallsite> callsites = FileOffsetMapper.getCallsiteFromOffset(
+                new File(bc.getBinaryFilePath()),
+                bc.getBuildId(),
+                bc.getOffset());
 
         if (callsites == null || Iterables.isEmpty(callsites)) {
             return null;