From 535c2e4f43a6791a20642648387294ee9e97b384 Mon Sep 17 00:00:00 2001 From: Nuno Lopes Date: Tue, 21 Nov 2023 02:07:15 +0000 Subject: [PATCH] implement based-on semantics of memory(argmem) attribute. This implementation doesn't take escaped pointers through ptr2int into account #969 --- ir/attrs.cpp | 4 ++++ ir/attrs.h | 4 +++- ir/globals.cpp | 1 + ir/globals.h | 1 + ir/instr.cpp | 20 ++++---------------- ir/memory.cpp | 11 ++++++++--- ir/pointer.cpp | 21 ++++++++++++++------- ir/pointer.h | 1 + tests/alive-tv/attrs/argmemonly4.srctgt.ll | 8 ++++++++ tests/alive-tv/attrs/argmemonly5.srctgt.ll | 12 ++++++++++++ tests/alive-tv/attrs/argmemonly6.srctgt.ll | 11 +++++++++++ tools/transform.cpp | 8 +++++++- 12 files changed, 74 insertions(+), 28 deletions(-) create mode 100644 tests/alive-tv/attrs/argmemonly4.srctgt.ll create mode 100644 tests/alive-tv/attrs/argmemonly5.srctgt.ll create mode 100644 tests/alive-tv/attrs/argmemonly6.srctgt.ll diff --git a/ir/attrs.cpp b/ir/attrs.cpp index cafacbe2c..661c67ceb 100644 --- a/ir/attrs.cpp +++ b/ir/attrs.cpp @@ -161,6 +161,10 @@ bool MemoryAccess::canOnlyWrite(AccessType ty) const { return canWrite(ty); } +bool MemoryAccess::canAccessAnything() const { + return canReadAnything() && canWriteAnything(); +} + bool MemoryAccess::canReadAnything() const { for (unsigned i = 0; i < NumTypes; ++i) { if (!canRead(AccessType(i))) diff --git a/ir/attrs.h b/ir/attrs.h index 0ec586b6c..355d284ab 100644 --- a/ir/attrs.h +++ b/ir/attrs.h @@ -29,6 +29,7 @@ class MemoryAccess final { bool canWrite(AccessType ty) const; bool canOnlyRead(AccessType ty) const; bool canOnlyWrite(AccessType ty) const; + bool canAccessAnything() const; bool canReadAnything() const; bool canWriteAnything() const; bool canReadSomething() const; @@ -64,7 +65,8 @@ class ParamAttrs final { NoUndef = 1<<6, Align = 1<<7, Returned = 1<<8, NoAlias = 1<<9, DereferenceableOrNull = 1<<10, AllocPtr = 1<<11, AllocAlign = 1<<12, - ZeroExt = 1<<13, SignExt = 1<<14, NoFPClass = 1<<15 }; + ZeroExt = 1<<13, SignExt = 1<<14, NoFPClass = 1<<15, + IsArg = 1<<16 }; ParamAttrs(unsigned bits = None) : bits(bits) {} diff --git a/ir/globals.cpp b/ir/globals.cpp index 369c5bf2b..96fa26f66 100644 --- a/ir/globals.cpp +++ b/ir/globals.cpp @@ -36,6 +36,7 @@ bool has_write_fncall = true; bool has_nocapture = true; bool has_noread = true; bool has_nowrite = true; +bool has_ptr_arg = true; bool has_null_block = true; bool null_is_dereferenceable = false; bool does_int_mem_access = true; diff --git a/ir/globals.h b/ir/globals.h index 4762b4f7e..9eed595fb 100644 --- a/ir/globals.h +++ b/ir/globals.h @@ -71,6 +71,7 @@ extern bool has_write_fncall; extern bool has_nocapture; extern bool has_noread; extern bool has_nowrite; +extern bool has_ptr_arg; /// Whether there null pointers appear in the program extern bool has_null_pointer; diff --git a/ir/instr.cpp b/ir/instr.cpp index 4cfebdc3b..d9912801d 100644 --- a/ir/instr.cpp +++ b/ir/instr.cpp @@ -2227,18 +2227,6 @@ static void eq_bids(OrExpr &acc, Memory &m, const Type &t, } } -static expr ptr_only_args(State &s, const Pointer &p) { - expr bid = p.getBid(); - auto &m = s.getMemory(); - - OrExpr e; - for (auto &in : s.getFn().getInputs()) { - if (hasPtr(in.getType())) - eq_bids(e, m, in.getType(), s[in], bid); - } - return e(); -} - static void check_can_load(State &s, const expr &p0) { auto &attrs = s.getFn().getFnAttrs(); if (attrs.mem.canReadAnything()) @@ -2251,10 +2239,10 @@ static void check_can_load(State &s, const expr &p0) { expr nonreadable = false; (attrs.mem.canRead(MemoryAccess::Globals) ? readable : nonreadable) - |= p.isWritableGlobal(); + |= p.isWritableGlobal() && !p.isBasedOnArg(); (attrs.mem.canRead(MemoryAccess::Args) ? readable : nonreadable) - |= ptr_only_args(s, p); + |= p.isBasedOnArg(); s.addUB(std::move(readable)); s.addUB(!nonreadable); @@ -2273,10 +2261,10 @@ static void check_can_store(State &s, const expr &p0) { expr nonwritable = false; (attrs.mem.canWrite(MemoryAccess::Globals) ? writable : nonwritable) - |= p.isWritableGlobal(); + |= p.isWritableGlobal() && !p.isBasedOnArg(); (attrs.mem.canWrite(MemoryAccess::Args) ? writable : nonwritable) - |= ptr_only_args(s, p); + |= p.isBasedOnArg(); s.addUB(std::move(writable)); s.addUB(!nonwritable); diff --git a/ir/memory.cpp b/ir/memory.cpp index cd2238b21..9d123b712 100644 --- a/ir/memory.cpp +++ b/ir/memory.cpp @@ -1294,9 +1294,12 @@ void Memory::markByVal(unsigned bid, bool is_const) { byval_blks.emplace_back(bid, is_const); } -expr Memory::mkInput(const char *name, const ParamAttrs &attrs) { +expr Memory::mkInput(const char *name, const ParamAttrs &attrs0) { unsigned max_bid = has_null_block + num_globals_src + next_ptr_input++; assert(max_bid < num_nonlocals_src); + + auto attrs = attrs0; + attrs.set(ParamAttrs::IsArg); Pointer p(*this, name, false, false, false, attrs); auto bid = p.getShortBid(); @@ -1314,8 +1317,8 @@ expr Memory::mkInput(const char *name, const ParamAttrs &attrs) { return std::move(p).release(); } -pair Memory::mkUndefInput(const ParamAttrs &attrs) const { - bool nonnull = attrs.has(ParamAttrs::NonNull); +pair Memory::mkUndefInput(const ParamAttrs &attrs0) const { + bool nonnull = attrs0.has(ParamAttrs::NonNull); unsigned log_offset = ilog2_ceil(bits_for_offset, false); unsigned bits_undef = bits_for_offset + nonnull * log_offset; expr undef = expr::mkFreshVar("undef", expr::mkUInt(0, bits_undef)); @@ -1329,6 +1332,8 @@ pair Memory::mkUndefInput(const ParamAttrs &attrs) const { one << var.zextOrTrunc(bits_for_offset)); offset = undef.extract(bits_undef - 1, log_offset) | shl; } + auto attrs = attrs0; + attrs.set(ParamAttrs::IsArg); return { Pointer(*this, expr::mkUInt(0, bits_for_bid), offset, attrs).release(), std::move(undef) }; diff --git a/ir/pointer.cpp b/ir/pointer.cpp index 844cb5672..7d3ddac19 100644 --- a/ir/pointer.cpp +++ b/ir/pointer.cpp @@ -45,6 +45,7 @@ static expr attr_to_bitvec(const ParamAttrs &attrs) { bits |= to_bit(has_nocapture, ParamAttrs::NoCapture); bits |= to_bit(has_noread, ParamAttrs::NoRead); bits |= to_bit(has_nowrite, ParamAttrs::NoWrite); + bits |= to_bit(has_ptr_arg, ParamAttrs::IsArg); return expr::mkUInt(bits, bits_for_ptrattrs); } @@ -621,17 +622,23 @@ expr Pointer::isNocapture(bool simplify) const { return p.extract(0, 0) == 1; } +#define GET_ATTR(attr, idx) \ + if (!attr) \ + return false; \ + unsigned idx_ = idx; \ + return p.extract(idx_, idx_) == 1; + expr Pointer::isNoRead() const { - if (!has_noread) - return false; - return p.extract(has_nocapture, has_nocapture) == 1; + GET_ATTR(has_noread, (unsigned)has_nocapture); } expr Pointer::isNoWrite() const { - if (!has_nowrite) - return false; - unsigned idx = (unsigned)has_nocapture + (unsigned)has_noread; - return p.extract(idx, idx) == 1; + GET_ATTR(has_nowrite, (unsigned)has_nocapture + (unsigned)has_noread); +} + +expr Pointer::isBasedOnArg() const { + GET_ATTR(has_ptr_arg, (unsigned)has_nocapture + (unsigned)has_noread + + (unsigned)has_nowrite); } Pointer Pointer::mkNullPointer(const Memory &m) { diff --git a/ir/pointer.h b/ir/pointer.h index 42ce7afb6..d675de3e4 100644 --- a/ir/pointer.h +++ b/ir/pointer.h @@ -121,6 +121,7 @@ class Pointer { smt::expr isNocapture(bool simplify = true) const; smt::expr isNoRead() const; smt::expr isNoWrite() const; + smt::expr isBasedOnArg() const; smt::expr refined(const Pointer &other) const; smt::expr fninputRefined(const Pointer &other, std::set &undef, diff --git a/tests/alive-tv/attrs/argmemonly4.srctgt.ll b/tests/alive-tv/attrs/argmemonly4.srctgt.ll new file mode 100644 index 000000000..d7c977a18 --- /dev/null +++ b/tests/alive-tv/attrs/argmemonly4.srctgt.ll @@ -0,0 +1,8 @@ +define i32 @src(ptr %0) memory(read, argmem: none) { + %2 = load i32, ptr %0, align 4 + ret i32 %2 +} + +define i32 @tgt(ptr %0) memory(read, argmem: none) { + unreachable +} diff --git a/tests/alive-tv/attrs/argmemonly5.srctgt.ll b/tests/alive-tv/attrs/argmemonly5.srctgt.ll new file mode 100644 index 000000000..c7f192ac2 --- /dev/null +++ b/tests/alive-tv/attrs/argmemonly5.srctgt.ll @@ -0,0 +1,12 @@ +@x = external global i8 + +define i8 @src(ptr %0) memory(read, argmem: none) { + %2 = load i8, ptr @x, align 1 + ret i8 %2 +} + +define i8 @tgt(ptr %0) memory(none) { + unreachable +} + +; ERROR: Source is more defined than target diff --git a/tests/alive-tv/attrs/argmemonly6.srctgt.ll b/tests/alive-tv/attrs/argmemonly6.srctgt.ll new file mode 100644 index 000000000..a68b4f81d --- /dev/null +++ b/tests/alive-tv/attrs/argmemonly6.srctgt.ll @@ -0,0 +1,11 @@ +@x = external global i8 + +define i8 @src(ptr %0) memory(read, argmem: none) { + %2 = load i8, ptr @x, align 1 + ret i8 %2 +} + +define i8 @tgt(ptr %0) memory(read, argmem: none) { + %2 = load i8, ptr @x, align 1 + ret i8 %2 +} diff --git a/tools/transform.cpp b/tools/transform.cpp index 2e4aed920..35a2135fd 100644 --- a/tools/transform.cpp +++ b/tools/transform.cpp @@ -955,6 +955,7 @@ static void calculateAndInitConstants(Transform &t) { observes_addresses = false; bool does_any_byte_access = false; has_indirect_fncalls = false; + has_ptr_arg = false; set inaccessiblememonly_fns; num_inaccessiblememonly_fns = 0; @@ -987,6 +988,8 @@ static void calculateAndInitConstants(Transform &t) { if (!i) continue; + has_ptr_arg |= hasPtr(i->getType()); + update_min_vect_sz(i->getType()); if (i->hasAttribute(ParamAttrs::Dereferenceable)) { @@ -1151,7 +1154,9 @@ static void calculateAndInitConstants(Transform &t) { has_nocapture = has_attr(ParamAttrs::NoCapture); has_noread = has_attr(ParamAttrs::NoRead); has_nowrite = has_attr(ParamAttrs::NoWrite); - bits_for_ptrattrs = has_nocapture + has_noread + has_nowrite; + has_ptr_arg &= !t.src.getFnAttrs().mem.canAccessAnything() || + !t.tgt.getFnAttrs().mem.canAccessAnything(); + bits_for_ptrattrs = has_nocapture + has_noread + has_nowrite + has_ptr_arg; // ceil(log2(maxblks)) + 1 for local bit bits_for_bid = max(1u, ilog2_ceil(max(num_locals, num_nonlocals), false)) @@ -1231,6 +1236,7 @@ static void calculateAndInitConstants(Transform &t) { << "\ndoes_mem_access: " << does_mem_access << "\ndoes_ptr_mem_access: " << does_ptr_mem_access << "\ndoes_int_mem_access: " << does_int_mem_access + << "\nhas_ptr_arg: " << has_ptr_arg << '\n'; }