summaryrefslogtreecommitdiff
path: root/cpu_ref
diff options
context:
space:
mode:
authorLuke Drummond <luke.drummond@codeplay.com>2017-04-17 11:47:06 -0700
committerDavid Gross <dgross@google.com>2017-04-19 11:50:51 -0700
commitb4b603d8c1c5574a7a76c7f74d03b996f0902e9a (patch)
treeea73354439fb063ad01e704945ca42d8fe9e9102 /cpu_ref
parenta8b6960a8815bb1fff74c1d9dcf4c572a7a831d9 (diff)
downloadrs-b4b603d8c1c5574a7a76c7f74d03b996f0902e9a.tar.gz
[debugger] make the ScriptGroup inspection point weak
Breakpoints set on named ScriptGroups were never resolved by the debugger due to aggressive optimizations by clang in release builds. `debugHintScriptGroup2`, used by the debugger to inspect ScriptGroup compilations at `optlevel == 0`, had its inner loop optimized away completely as there are no visible side effects (where the ALOGV calls are actually empty macros). This was fixed by using an `__asm__ __volatile__ ("")` construct as per the gcc docs: noinline This function attribute prevents a function from being considered for inlining. If the function does not have side-effects, there are optimizations other than inlining that causes function calls to be optimized away, although the function call is live. To keep such calls from being optimized away, put `asm ("")` in the called function, to serve as a special side-effect. (see also https://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes) Although preventing the inner loop from being optimized away is enough to get the function called, it is apparent that clang only sets up the call frame with arguments that it can see are used inside the function body, meaning that we had garbage values for `groupName` and `groupNameSize` unless the symbol is weakly visible so that the compiler can no longer introspect that the arguments are ignored. Test: aosp_x86_64-eng (emulator) - RsTest (32-bit, 64-bit) - cts -m RenderscriptTest - rs/lldb (required various other pending CLs) Bug: 23535482 Change-Id: I24e23895e67d1ede8bf8e671165a3a6843a11082 Signed-off-by: Luke Drummond <luke.drummond@codeplay.com> (cherry picked from commit 6e64e51a6a252bd88acfa72dac8372c863f524e0)
Diffstat (limited to 'cpu_ref')
-rw-r--r--cpu_ref/rsCpuScriptGroup2.cpp18
1 files changed, 17 insertions, 1 deletions
diff --git a/cpu_ref/rsCpuScriptGroup2.cpp b/cpu_ref/rsCpuScriptGroup2.cpp
index e42efe18..11393ff4 100644
--- a/cpu_ref/rsCpuScriptGroup2.cpp
+++ b/cpu_ref/rsCpuScriptGroup2.cpp
@@ -330,7 +330,22 @@ void generateSourceSlot(RsdCpuReferenceImpl* ctxt,
} // anonymous namespace
-extern __attribute__((noinline))
+// This function is used by the debugger to inspect ScriptGroup
+// compilations.
+//
+// "__attribute__((noinline))" and "__asm__" are used to prevent the
+// function call from being eliminated as a no-op (see the "noinline"
+// attribute in gcc documentation).
+//
+// "__attribute__((weak))" is used to prevent callers from recognizing
+// that this is guaranteed to be the function definition, recognizing
+// that certain arguments are unused, and optimizing away the passing
+// of those arguments (see the LLVM optimization
+// DeadArgumentElimination). Theoretically, the compiler could get
+// aggressive enough with link-time optimization that even marking the
+// entry point as a weak definition wouldn't solve the problem.
+//
+extern __attribute__((noinline)) __attribute__((weak))
void debugHintScriptGroup2(const char* groupName,
const uint32_t groupNameSize,
const ExpandFuncTy* kernel,
@@ -338,6 +353,7 @@ void debugHintScriptGroup2(const char* groupName,
ALOGV("group name: %d:%s\n", groupNameSize, groupName);
for (uint32_t i=0; i < kernelCount; ++i) {
const char* f1 = (const char*)(kernel[i]);
+ __asm__ __volatile__("");
ALOGV(" closure: %p\n", (const void*)f1);
}
// do nothing, this is just a hook point for the debugger.