fix #1028: the align attribute on load, store, etc implies dereferenc…

…eability We had a partial implementation for the ASM mode; this patch superseeds that one
AliveToolkit · Apr 28, 2024 · 31a8bf1 · 31a8bf1
1 parent 61fe61b
commit 31a8bf1
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 55 deletions.
diff --git a/ir/instr.cpp b/ir/instr.cpp
@@ -3893,7 +3893,7 @@ DEFINE_AS_RETZEROALIGN(Load, getMaxAllocSize);
 DEFINE_AS_RETZERO(Load, getMaxGEPOffset);
 
 uint64_t Load::getMaxAccessSize() const {
-  return Memory::getStoreByteSize(getType());
+  return round_up(Memory::getStoreByteSize(getType()), align);
 }
 
 MemInstr::ByteAccessInfo Load::getByteAccessInfo() const {
@@ -3939,7 +3939,7 @@ DEFINE_AS_RETZEROALIGN(Store, getMaxAllocSize);
 DEFINE_AS_RETZERO(Store, getMaxGEPOffset);
 
 uint64_t Store::getMaxAccessSize() const {
-  return Memory::getStoreByteSize(val->getType());
+  return round_up(Memory::getStoreByteSize(val->getType()), align);
 }
 
 MemInstr::ByteAccessInfo Store::getByteAccessInfo() const {
@@ -3992,7 +3992,7 @@ DEFINE_AS_RETZEROALIGN(Memset, getMaxAllocSize);
 DEFINE_AS_RETZERO(Memset, getMaxGEPOffset);
 
 uint64_t Memset::getMaxAccessSize() const {
-  return getIntOr(*bytes, UINT64_MAX);
+  return round_up(getIntOr(*bytes, UINT64_MAX), align);
 }
 
 MemInstr::ByteAccessInfo Memset::getByteAccessInfo() const {
@@ -4160,7 +4160,7 @@ DEFINE_AS_RETZEROALIGN(Memcpy, getMaxAllocSize);
 DEFINE_AS_RETZERO(Memcpy, getMaxGEPOffset);
 
 uint64_t Memcpy::getMaxAccessSize() const {
-  return getIntOr(*bytes, UINT64_MAX);
+  return round_up(getIntOr(*bytes, UINT64_MAX), max(align_src, align_dst));
 }
 
 MemInstr::ByteAccessInfo Memcpy::getByteAccessInfo() const {

diff --git a/ir/memory.cpp b/ir/memory.cpp
@@ -1783,7 +1783,8 @@ Memory::alloc(const expr *size, uint64_t align, BlockKind blockKind,
   expr size_zext;
   expr nooverflow = true;
   if (size) {
-    size_zext  = size->zextOrTrunc(bits_size_t);
+    size_zext  = size->zextOrTrunc(bits_size_t)
+                      .round_up(expr::mkUInt(align, bits_size_t));
     nooverflow = size->bits() <= bits_size_t ? true :
                    size->extract(size->bits()-1, bits_size_t) == 0;
   }

diff --git a/ir/pointer.cpp b/ir/pointer.cpp
@@ -428,8 +428,10 @@ static pair<expr, expr> is_dereferenceable(Pointer &p,
 pair<AndExpr, expr>
 Pointer::isDereferenceable(const expr &bytes0, uint64_t align,
                            bool iswrite, bool ignore_accessability) {
-  expr bytes_off = bytes0.zextOrTrunc(bits_for_offset);
-  expr bytes = bytes0.zextOrTrunc(bits_size_t);
+  expr bytes = bytes0.zextOrTrunc(bits_size_t)
+                     .round_up(expr::mkUInt(align, bits_size_t));
+  expr bytes_off = bytes.zextOrTrunc(bits_for_offset);
+
   DisjointExpr<expr> UB(expr(false)), is_aligned(expr(false)), all_ptrs;
 
   for (auto &[ptr_expr, domain] : DisjointExpr<expr>(p, 3)) {

diff --git a/smt/expr.cpp b/smt/expr.cpp
@@ -1117,6 +1117,11 @@ expr expr::abs() const {
   return mkIf(sge(mkUInt(0, s)), *this, mkInt(-1, s) * *this);
 }
 
+expr expr::round_up(const expr &power_of_two) const {
+  expr minus_1 = power_of_two - mkUInt(1, power_of_two);
+  return (*this + minus_1) & ~minus_1;
+}
+
 #define fold_fp_neg(fn)                                  \
   do {                                                   \
   expr cond, neg, v, v2;                                 \

diff --git a/smt/expr.h b/smt/expr.h
@@ -218,6 +218,8 @@ class expr {
 
   expr abs() const;
 
+  expr round_up(const expr &power_of_two) const;
+
   expr isNaN() const;
   expr isInf() const;
   expr isFPZero() const;

diff --git a/tests/alive-tv/attrs/align-callsite-withnoundef.srctgt.ll b/tests/alive-tv/attrs/align-callsite-withnoundef.srctgt.ll
@@ -1,3 +1,5 @@
+; ERROR: Source is more defined than target
+
 define void @src(ptr %p) {
   call void @g(ptr %p)
   load i8, ptr %p

diff --git a/tests/alive-tv/attrs/align-callsite2-withnoundef.srctgt.ll b/tests/alive-tv/attrs/align-callsite2-withnoundef.srctgt.ll
@@ -1,3 +1,5 @@
+; ERROR: Source is more defined than target
+
 define void @src(ptr %p) {
   call void @g(ptr align(4) %p)
   load i8, ptr %p

diff --git a/tests/alive-tv/attrs/align-callsite3-withnoundef.srctgt.ll b/tests/alive-tv/attrs/align-callsite3-withnoundef.srctgt.ll
@@ -1,3 +1,5 @@
+; ERROR: Source is more defined than target
+
 define void @src(ptr %p) {
   call void @g(ptr align(4) %p)
   load i8, ptr %p

diff --git a/tests/alive-tv/attrs/align-callsite4-withnoundef.srctgt.ll b/tests/alive-tv/attrs/align-callsite4-withnoundef.srctgt.ll
@@ -0,0 +1,13 @@
+define void @src(ptr %p) {
+  call void @g(ptr %p)
+  load i8, ptr %p
+  ret void
+}
+
+define void @tgt(ptr %p) {
+  call void @g(ptr %p)
+  load i8, ptr %p, align 4
+  ret void
+}
+
+declare void @g(ptr align(4) dereferenceable(4) noundef)
diff --git a/tests/alive-tv/attrs/dereferenceable-fndef-heap3.srctgt.ll b/tests/alive-tv/attrs/dereferenceable-fndef-heap3.srctgt.ll
@@ -1,9 +1,9 @@
-define dereferenceable(4) ptr @src() {
+define dereferenceable(90) ptr @src() {
   %p = call ptr @malloc(i64 3)
   ret ptr %p
 }
 
-define dereferenceable(4) ptr @tgt() {
+define dereferenceable(90) ptr @tgt() {
   unreachable
 }
 

diff --git a/tests/alive-tv/bugs/pr3720.srctgt.ll b/tests/alive-tv/bugs/pr3720.srctgt.ll
@@ -1,22 +1,14 @@
-%struct.st = type <{ i16 }>
-
 define void @src(ptr %p) {
-entry:
-        %s = alloca %struct.st, align 4
-        %0 = getelementptr %struct.st, ptr %s, i32 0, i32 0
-        store i16 1, ptr %0, align 4
-        %s1 = bitcast ptr %s to ptr
-        call void @llvm.memcpy.i32(ptr %p, ptr %s1, i32 2, i32 1)
+        %s = alloca i16, align 4
+        store i16 1, ptr %s
+        call void @llvm.memcpy.i32(ptr %p, ptr %s, i32 2, i32 1)
         ret void
 }
 
 declare void @llvm.memcpy.i32(ptr nocapture, ptr nocapture, i32, i32)
 
 define void @tgt(ptr %p) {
-entry:
-        %p1 = bitcast ptr %p to ptr
-        %p1.0 = getelementptr %struct.st, ptr %p1, i32 0, i32 0
-        store i16 1, ptr %p1.0
+        store i16 1, ptr %p
         ret void
 }
 

diff --git a/tests/alive-tv/memory/align.src.ll b/tests/alive-tv/memory/align.src.ll
@@ -1,3 +1,5 @@
+; ERROR: Source is more defined than target
+
 define i32 @foo1(ptr %a) {
   %v = load i32, ptr %a, align 4
   %ptrint = ptrtoint ptr %a to i64

diff --git a/tests/alive-tv/memory/nullptr-align.srctgt.ll b/tests/alive-tv/memory/nullptr-align.srctgt.ll
@@ -1,3 +1,5 @@
+; ERROR: Source is more defined than target
+
 define i8 @src() null_pointer_is_valid {
   %v = load i8, ptr null, align 4
   ret i8 %v

diff --git a/tools/transform.cpp b/tools/transform.cpp
@@ -1627,53 +1627,21 @@ void Transform::preprocess() {
   if (config::tgt_is_asm) {
     tgt.getFnAttrs().set(FnAttrs::Asm);
 
-    // all memory blocks are considered to have a size multiple of alignment
-    // since asm memory accesses won't trap as it won't cross the page boundary
-    vector<pair<const Instr*, unique_ptr<Instr>>> to_add;
+    // there are no #side-effect things in assembly
     vector<Instr*> to_remove;
     for (auto &bb : src.getBBs()) {
       for (auto &i : bb->instrs()) {
-        if (auto *load = dynamic_cast<const Load*>(&i)) {
-          auto align = load->getAlign();
-          auto size  = load->getMaxAccessSize();
-          if (size % align) {
-            static IntType i64("i64", 64);
-            auto bytes = make_unique<IntConst>(i64, round_up(size, align));
-            to_add.emplace_back(load, make_unique<Assume>(
-              vector<Value*>{&load->getPtr(), bytes.get()},
-              Assume::Dereferenceable));
-            src.addConstant(std::move(bytes));
-          }
-        } else if (auto *call = dynamic_cast<const FnCall*>(&i)) {
+        if (auto *call = dynamic_cast<const FnCall*>(&i)) {
           if (call->getFnName() == "#sideeffect") {
             to_remove.emplace_back(const_cast<Instr*>(&i));
           }
         }
       }
-      for (auto &[i, assume] : to_add) {
-        bb->addInstrAt(std::move(assume), i, false);
-      }
       for (auto &i : to_remove) {
         bb->delInstr(i);
       }
-      to_add.clear();
       to_remove.clear();
     }
-
-    // increase size of global variables to be a multiple of alignment
-    for (auto gv : tgt.getGlobalVars()) {
-      if (gv->isArbitrarySize())
-        continue;
-      auto align = gv->getAlignment();
-      auto sz = gv->size();
-      auto newsize = round_up(sz, align);
-      if (newsize != sz) {
-        gv->increaseSize(newsize);
-        if (auto src_gv = dynamic_cast<GlobalVariable*>(
-            src.getGlobalVar(gv->getName())))
-          src_gv->increaseSize(newsize);
-      }
-    }
   }
 
   remove_unreachable_bbs(src);