[profiler] fix 4 bugs in --prof/linux-tick-processor

- In log.cc, we used InstructionStart() for code create events, but
  the Code object address for code move events. Change to use
  InstructionStart() for both.
- The symbol table contains some kind of virtual address, not file
  offsets. They happened to be identical in the past but are no longer,
  probably due to toolchain changes. Now we use objdump to figure out
  the difference between virtual addresses and file offsets.
- When a new code object happened to be created at the same address as
  a previous one, we wouldn't update it.
  This is indeed wrong, as predicted in a TODO by Jaro.
- For 64bit addresses, using >>> is wrong, now replaced with division.


Change-Id: Ib23114ed736f98bfc33c65004a039a3fd04d3c49
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2016586Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66145}

src/logging/log.cc

@@ -1355,8 +1355,8 @@ void Logger::RegExpCodeCreateEvent(Handle<AbstractCode> code,
 
 void Logger::CodeMoveEvent(AbstractCode from, AbstractCode to) {
   if (!is_listening_to_code_events()) return;
-  MoveEventInternal(CodeEventListener::CODE_MOVE_EVENT, from.address(),
-                    to.address());
+  MoveEventInternal(CodeEventListener::CODE_MOVE_EVENT, from.InstructionStart(),
+                    to.InstructionStart());
 }
 
 void Logger::SharedFunctionInfoMoveEvent(Address from, Address to) {

tools/codemap.js

@@ -140,7 +140,7 @@ CodeMap.prototype.addStaticCode = function(
 CodeMap.prototype.markPages_ = function(start, end) {
   for (var addr = start; addr <= end;
        addr += CodeMap.PAGE_SIZE) {
-    this.pages_[addr >>> CodeMap.PAGE_ALIGNMENT] = 1;
+    this.pages_[(addr / CodeMap.PAGE_SIZE)|0] = 1;
   }
 };
 
@@ -187,7 +187,7 @@ CodeMap.prototype.findInTree_ = function(tree, addr) {
  * @param {number} addr Address.
  */
 CodeMap.prototype.findAddress = function(addr) {
-  var pageAddr = addr >>> CodeMap.PAGE_ALIGNMENT;
+  var pageAddr = (addr / CodeMap.PAGE_SIZE)|0;
   if (pageAddr in this.pages_) {
     // Static code entries can contain "holes" of unnamed code.
     // In this case, the whole library is assigned to this address.

tools/profile.js

@@ -169,8 +169,12 @@ Profile.prototype.addFuncCode = function(
     if (entry.size === size && entry.func === func) {
       // Entry state has changed.
       entry.state = state;
+    } else {
+      this.codeMap_.deleteCode(start);
+      entry = null;
     }
-  } else {
+  }
+  if (!entry) {
     entry = new Profile.DynamicFuncCodeEntry(size, type, func, state);
     this.codeMap_.addCode(start, entry);
   }
@@ -935,14 +939,16 @@ JsonProfile.prototype.addFuncCode = function(
   // TODO(jarin): Insert the code object into the SFI's code list.
   var entry = this.codeMap_.findDynamicEntryByStartAddress(start);
   if (entry) {
-    // TODO(jarin) This does not look correct, we should really
-    // update the code object (remove the old one and insert this one).
     if (entry.size === size && entry.func === func) {
       // Entry state has changed.
       entry.state = state;
+    } else {
+      this.codeMap_.deleteCode(start);
+      entry = null;
     }
-  } else {
-    var entry = new CodeMap.CodeEntry(size, name, 'JS');
+  }
+  if (!entry) {
+    entry = new CodeMap.CodeEntry(size, name, 'JS');
     this.codeMap_.addCode(start, entry);
 
     entry.codeId = this.codeEntries_.length;

tools/tickprocessor-driver.js

@@ -62,7 +62,7 @@ if (params.sourceMap) {
   sourceMap = SourceMap.load(params.sourceMap);
 }
 var tickProcessor = new TickProcessor(
-  new (entriesProviders[params.platform])(params.nm, params.targetRootFS,
+  new (entriesProviders[params.platform])(params.nm, params.objdump, params.targetRootFS,
                                           params.apkEmbeddedLibrary),
   params.separateIc,
   params.separateBytecodes,

tools/tickprocessor.js

@@ -706,10 +706,14 @@ CppEntriesProvider.prototype.parseNextLine = function() {
 };
 
 
-function UnixCppEntriesProvider(nmExec, targetRootFS, apkEmbeddedLibrary) {
+function UnixCppEntriesProvider(nmExec, objdumpExec, targetRootFS, apkEmbeddedLibrary) {
   this.symbols = [];
+  // File offset of a symbol minus the virtual address of a symbol found in
+  // the symbol table.
+  this.fileOffsetMinusVma = 0;
   this.parsePos = 0;
   this.nmExec = nmExec;
+  this.objdumpExec = objdumpExec;
   this.targetRootFS = targetRootFS;
   this.apkEmbeddedLibrary = apkEmbeddedLibrary;
   this.FUNC_RE = /^([0-9a-fA-F]{8,16}) ([0-9a-fA-F]{8,16} )?[tTwW] (.*)$/;
@@ -731,6 +735,14 @@ UnixCppEntriesProvider.prototype.loadSymbols = function(libName) {
       os.system(this.nmExec, ['-C', '-n', '-S', libName], -1, -1),
       os.system(this.nmExec, ['-C', '-n', '-S', '-D', libName], -1, -1)
     ];
+
+    const objdumpOutput = os.system(this.objdumpExec, ['-h', libName], -1, -1);
+    for (const line of objdumpOutput.split('\n')) {
+      const [,sectionName,,vma,,fileOffset] = line.trim().split(/\s+/);
+      if (sectionName === ".text") {
+        this.fileOffsetMinusVma = parseInt(fileOffset, 16) - parseInt(vma, 16);
+      }
+    }
   } catch (e) {
     // If the library cannot be found on this system let's not panic.
     this.symbols = ['', ''];
@@ -754,7 +766,7 @@ UnixCppEntriesProvider.prototype.parseNextLine = function() {
   var fields = line.match(this.FUNC_RE);
   var funcInfo = null;
   if (fields) {
-    funcInfo = { name: fields[3], start: parseInt(fields[1], 16) };
+    funcInfo = { name: fields[3], start: parseInt(fields[1], 16) + this.fileOffsetMinusVma };
     if (fields[2]) {
       funcInfo.size = parseInt(fields[2], 16);
     }
@@ -763,8 +775,8 @@ UnixCppEntriesProvider.prototype.parseNextLine = function() {
 };
 
 
-function MacCppEntriesProvider(nmExec, targetRootFS, apkEmbeddedLibrary) {
-  UnixCppEntriesProvider.call(this, nmExec, targetRootFS, apkEmbeddedLibrary);
+function MacCppEntriesProvider(nmExec, objdumpExec, targetRootFS, apkEmbeddedLibrary) {
+  UnixCppEntriesProvider.call(this, nmExec, objdumpExec, targetRootFS, apkEmbeddedLibrary);
   // Note an empty group. It is required, as UnixCppEntriesProvider expects 3 groups.
   this.FUNC_RE = /^([0-9a-fA-F]{8,16})() (.*)$/;
 };
@@ -786,7 +798,7 @@ MacCppEntriesProvider.prototype.loadSymbols = function(libName) {
 };
 
 
-function WindowsCppEntriesProvider(_ignored_nmExec, targetRootFS,
+function WindowsCppEntriesProvider(_ignored_nmExec, _ignored_objdumpExec, targetRootFS,
                                    _ignored_apkEmbeddedLibrary) {
   this.targetRootFS = targetRootFS;
   this.symbols = '';
@@ -909,6 +921,8 @@ class ArgumentsProcessor extends BaseArgumentsProcessor {
           'Specify that we are running on Mac OS X platform'],
       '--nm': ['nm', 'nm',
           'Specify the \'nm\' executable to use (e.g. --nm=/my_dir/nm)'],
+      '--objdump': ['objdump', 'objdump',
+          'Specify the \'objdump\' executable to use (e.g. --objdump=/my_dir/objdump)'],
       '--target': ['targetRootFS', '',
           'Specify the target root directory for cross environment'],
       '--apk-embedded-library': ['apkEmbeddedLibrary', '',
@@ -951,6 +965,7 @@ class ArgumentsProcessor extends BaseArgumentsProcessor {
       preprocessJson: null,
       targetRootFS: '',
       nm: 'nm',
+      objdump: 'objdump',
       range: 'auto,auto',
       distortion: 0,
       timedRange: false,