disable "Dissasembly" function for inlined functions (size calculation of inline functions not possible)

[GitMensch]

Describe the bug The size is definitely wrong (the inlined code consists of two 8 byte memcmp which are both done and a function call that is never executed). This leads to not usable disassembly view:

To Reproduce Steps to reproduce the behavior:

1. Use a recording that has a lot of calls of an inlined function at several places within a huge function; including inlining at the calling functions start and end.
2. Use disassembly feature
3. wait for some time, then see timeout (before the changes fixing #542 actually a SIGSEGV)
4. see (with #535 applied) a huge size calculated for the inlined function

Expected behavior Open to discussion; the actual disassembly for inlined functions possibly could vary (I'm not sure), otherwise either disassemble the inlined version that has most cost (if we know that), otherwise the first one

[milianw]

I fear without an MWE we can't really invesetigate this. You'll have to debug this yourself

[GitMensch]

I try, can you please update HACKING on how to correctly set this beast up for debugging? I've manually adjusted a bunch of cmake files to get "something" reasonable, but this must be easier...

[Also needed: a note about how to run the tests and add new ones.]

[milianw]

what do you mean exactly, i.e. what's your problem? Once you can compile it you can debug it, no? Adjusting cmake files cannot be correct, but without knowing what you are trying to do I cannot give you any hint on what the right approach would but. This is just a totally normal cmake project after all, there's zero special about hotspot in any regard. Are you maybe struggling with cmake in general? Just set CMAKE_BUILD_TYPE=Debug and you should be all set

[GitMensch]

While this is just a example it possibly shows the problem:

#include <stdlib.h>
#include <string.h>

static void __inline __attribute__((always_inline)) do_check (char *c)
{
   if (!memcmp(c, "abcd", 4)
    || !memcmp(c, "ABCD", 4)
    || !memcmp(c, "DCBA", 4)
    || !memcmp(c, "dcba", 4)) {
      exit (1);
   }
}

#define code(pos)  \
   for (int i = 0; i < 99999; i++) {  \
      checkme[pos] = (char)i;         \
      do_check(checkme);              \
   }

#define codes   code(1) code (2) code (3) code (0)

#define codes2  codes codes codes codes codes codes codes
#define codes3  codes2 codes2 codes2 codes2 codes2 codes2
#define codes4  codes3 codes3 codes3 codes3 codes3 codes3

int main () {
   char checkme[4] = { 99, 98, 97, 96};

   for (int i = 0; i < 9999999; i++) {
      checkme[3] = (char)i;
      do_check(checkme);
   }

   codes4

   do_check(checkme);
   return 0;
}

$> gcc mega.c -ggdb3
$> perf record --call-graph dwarf,28192 --aio -z --mmap-pages 16M ./a.out
$> hotspot

Then use disassembly on main::do_check. You will see that it takes a while and you have a much bigger disassembly than maybe expected - you see nearly (?) the complete program.

[GitMensch]

Note: I've also not recognized any cycles in the disassembly (but more or less the expected output in the cycles within the source view).

[GitMensch]

@milianw Can you reproduce it (= is that enough of a minimal reproducible example)?

[milianw]

I can reproduce it, but is this really a bug? The code is inlined after all, and thus basically the whole app is part of do_check, no?

We could disable the disassembly support for inlined functions, which is I believe something that @lievenhey suggested to do anyways. But I don't see any alternative to this case here - what do you have in mind @GitMensch ?

[GitMensch]

As most commonly the inlined functions will result in the same generated assembler, I suggest to:

• inline it within the functions that use this (this is already the case, I think @lievenhey works on an option to "collapse" them
• when explicit requested: go easy and show the disassembly for the first inlined version, or - if possible - the disassembly for the inlined version which address range has the highest cost

[milianw]

Your assumption is completely wrong. Once a function gets inlined, all bets are off - there are most definitely no guarantees that the produced assembly is the same. actually, quite the contrary, the idea is to let the context of the parent function allow for further optimizations that would drastically change the inlined code.

furthermore, the compiler cannot give us a range for an inlined function, as it doesn't have any clear boundaries anymore.

what you are requesting is simply completely at odds with reality

[GitMensch]

If we can't get the range of a single inline, then how would @lievenhey be able to collapse them?

As noted already in the issue starter, the idea is

either disassemble the inlined version that has most cost (if we know that), otherwise the first one

Note that this "only" applies to the inline function in a single function (in the sample down as main::do_check), not to other inline versions as sub::do_check.

If this really isn't possible, then we may should disable the disassembly of the inline function itself, the GUI could ask "Not possible, do you want to disassemble the containing function 'main' instead?"

[lievenhey]

I use the debug info annotations. They have references to the source code (and in case of inlined functions they mark the function as inlined). But that doesn't really help in your case, since the compiler can also reorder code which can spread out the inlined function like this:

line of of foo()
line 1 of inlined bar()
line 2 of foo()
line 2 of inlined bar()
line 3 of foo()

As @milianw said, inlined functions throw off all bets, since the compiler can also remove parts of the inlined function if he can assure that these paths would never be executed. If you have a setup like this:

inline int foo(bool b) {
   if (b) return 2;
   return 4;
}

int bar() {
  return foo(false) + foo(true);
}

Under the assumption that bar will not be optimized the resulting code could look like this:

bar:
   mov %rax, 4
   mov %rbx, 2
   add %rax, %rax, %rbx
   ret

As you can see the branch was completely eliminated by the compiler.

[lievenhey]

And don't forget that inlined functions can call other inlined functions, which will get even more messy

[GitMensch]

I've adjusted the issue title. As you both said: showing the disassembly of inlined functions is only possible within another function. Therefore: please disable that, possible hinting to the user that he may disassemble a function where this inlined function is included.