[
https://issues.apache.org/jira/browse/IMPALA-12306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17786435#comment-17786435
]
ASF subversion and git services commented on IMPALA-12306:
----------------------------------------------------------
Commit db5f3b18e403865b9ce592b020b99f88669c4ac6 in impala's branch
refs/heads/master from Daniel Becker
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=db5f3b18e ]
IMPALA-12306: (Part 2) Make codegen cache tests with symbol emitter more robust
The codegen cache tests that include having a symbol emitter (previously
TestCodegenCache.{test_codegen_cache_with_asm_module_dir,test_codegen_cache_with_perf_map})
introduced by IMPALA-12260 were added to ensure we don't produce a
use-after-free.
There are two problems with these tests:
1. Setting the codegen cache size correctly in the tests has proved to
be difficult because new commits and different build types (debug
vs. release) have a huge effect on what sizes are appropriate. We
have had many build failures because of this.
2. Use-after-free is undefined behaviour and does not guarantee a
crash but the tests rely on the crash to catch the bug described in
IMPALA-12260.
This change solves the second problem. The tests added by IMPALA-12260
relied on a crash in the situation described there:
'LlvmCodeGen::symbol_emitter_' is registered as an event listener with
the current 'llvm::ExecutionEngine', then the engine is cached but the
'LlvmCodeGen' object, which owns the symbol emitter, is destroyed at the
end of the query. When the cached execution engine is destroyed later,
it frees any remaining object files and notifies the symbol emitter
about this, but the symbol emitter has already been destroyed so its
pointer is invalid (use-after-free).
However, we can't rely on the crash to detect the use-after-free because
1) the crash is not guaranteed to happen, use-after-free is undefined
behaviour
2) the crash may happen well after the query has finished returning
results.
This change solves the problem in the following way:
In 'CodegenSymbolEmitter' we introduce a counter that is incremented in
NotifyObjectEmitted() and decremented in NotifyFreeingObject(). At the
time of the destruction of the 'CodegenSymbolEmitter', this counter
should be zero - if it is greater than zero, the LLVM execution engine
to which the 'CodegenSymbolEmitter' is subscribed is still alive and it
will try to notify the symbol emitter when the object file is freed
(most likely when the execution engine itself is destroyed), leading to
use-after-free
We also add a hidden startup flag,
'--codegen_symbol_emitter_log_successful_destruction_test_only'. When it
is set to true, 'CodegenSymbolEmitter' will log a message when it is
being destroyed correctly (i.e. when the counter is zero and
use-after-free will not happen). We use it in the tests - if we don't
have the expected message in the logs (after some timeout), the test
fails.
Testing:
- modified the tests
TestCodegenCache.{test_codegen_cache_with_asm_module_dir,test_codegen_cache_with_perf_map}
so they reliably detect use-after-free.
Change-Id: I61b9b0de9c896f3de7eb1be7de33d822b1ab70d0
Reviewed-on: http://gerrit.cloudera.org:8080/20318
Reviewed-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
> Make codegen cache tests with symbol emitter more robust
> --------------------------------------------------------
>
> Key: IMPALA-12306
> URL: https://issues.apache.org/jira/browse/IMPALA-12306
> Project: IMPALA
> Issue Type: Bug
> Reporter: Daniel Becker
> Assignee: Daniel Becker
> Priority: Major
>
> The tests
> TestCodegenCache.{test_codegen_cache_with_asm_module_dir,test_codegen_cache_with_perf_map}
> introduced by
> [IMPALA-12260|http://issues.apache.org/jira/browse/IMPALA-12260] were added
> to ensure we don't crash because of a use-after-free. However, use-after-free
> is undefined behaviour and does not guarantee a crash, so the tests don't
> necessarily crash without the fix of
> [IMPALA-12260|http://issues.apache.org/jira/browse/IMPALA-12260]. We should
> find a way to make these tests detect the conditions that lead to this
> use-after-free.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org
For additional commands, e-mail: issues-all-h...@impala.apache.org
