[PATCH] Disable inlining between sanitized and non-sanitized functions

Chandler Carruth chandlerc at gmail.com
Wed Aug 7 02:01:18 PDT 2013


  Code looks great. Just nit picking the test case.


================
Comment at: test/Transforms/Inline/attributes.ll:4
@@ +3,3 @@
+
+; Test that different values of sanitizer attribute prevent inlining.
+
----------------
Rather than this generic comment that isn't terribly clear, I would document the specific invariant you're checking where you enforce it. For example:

    ; Check that an alwaysinline sanitized callee is still inlined
    call void test_alwaysinline_sanitize_address_callee()
  ; CHECK-NOT: call void test_alwaysinline_sanitize_address_callee




As I've done here, you should also name the test functions after them: test_no_attr_callee, test_no_attr_caller, test_sanitize_address_callee, etc.

================
Comment at: test/Transforms/Inline/attributes.ll:20
@@ +19,3 @@
+
+define i32 @test2f(i32 %i) sanitize_address {
+        ret i32 %i
----------------
I would set up all your callee functions at the top, because you're going to end up doing some sparse matrix of testing, and that makes it clear what the set of callees is for each caller.

================
Comment at: test/Transforms/Inline/attributes.ll:7
@@ +6,3 @@
+define i32 @test1f(i32 %i) {
+        ret i32 %i
+}
----------------
nit: 2-space indent...

================
Comment at: test/Transforms/Inline/attributes.ll:14
@@ +13,3 @@
+        ret i32 %Y
+; CHECK: @test1(
+; CHECK-NEXT: %Y = add i32 7, %W
----------------
Use the new CHECK-LABEL

================
Comment at: test/Transforms/Inline/attributes.ll:39
@@ +38,3 @@
+define i32 @test3(i32 %W) {
+        %X = call i32 @test3f(i32 7)
+        %Y = add i32 %X, %W
----------------
I would test all of the callee cases you're interested in within a single caller.

So have a no-attr caller that calls all the various callee combinations of interest, and checks each of them (in whichever direction). Does that make sense?


http://llvm-reviews.chandlerc.com/D1035



More information about the llvm-commits mailing list