Skip to content

Patched #1242#1293

Open
Yohello1 wants to merge 7 commits into
AliveToolkit:masterfrom
Yohello1:master
Open

Patched #1242#1293
Yohello1 wants to merge 7 commits into
AliveToolkit:masterfrom
Yohello1:master

Conversation

@Yohello1
Copy link
Copy Markdown

@Yohello1 Yohello1 commented Mar 5, 2026

Hello, Someone I know sent me this and said I should try patching it, took a quick jab at it and came up with this. Let me know if I missed anything, or am not properly following the coding standards.

Thank you,
Yohello1

Comment thread ir/memory.cpp Outdated
@@ -0,0 +1,13 @@
; EXPECT: ERROR: Source is more defined than target
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name of the test is not very good. this is testing that the load is not UB.
You need another test that shows that the return value can be replaced with poison. Also, run the tests; I believe some tests will fail with your change.

Comment thread ir/pointer.cpp Outdated
@Yohello1
Copy link
Copy Markdown
Author

Yohello1 commented Apr 1, 2026

Hello, I made the changes, ran the tests, and got 82% failure rate... so be back in a bit.

@Yohello1
Copy link
Copy Markdown
Author

Yohello1 commented Apr 3, 2026

@nunoplopes Hi, I got the tests working and did find that a decent amount did fail. But it took me a while to figure out that Im supposed to run ninja check to run the tests, I also don't see it documented anywhere. Should we document it maybe in the README.md?

On the note of tests, here are the changes:

FIXED (1):

  • alive-tv/attrs/nocapture.src.ll

NEW FAILURES (5):

  • alive-tv/attrs/readnone-fail.srctgt.ll
  • alive-tv/bugs/pr23599.srctgt.ll
  • alive-tv/calls/escape2.srctgt.ll
  • alive-tv/calls/ptr-store.srctgt.ll
  • alive-tv/lifetime-load-poison.srctgt.ll

@nunoplopes
Copy link
Copy Markdown
Member

@nunoplopes Hi, I got the tests working and did find that a decent amount did fail. But it took me a while to figure out that Im supposed to run ninja check to run the tests, I also don't see it documented anywhere. Should we document it maybe in the README.md?

probably 😅

@Yohello1
Copy link
Copy Markdown
Author

Hi @nunoplopes I think I've fixed all of the tests which had issues, can you maybe look over it and let me know if it's good or not? I can squash the merges if they look good

Comment thread README.md

Alive2's `opt` and `clang` translation validation requires a build of LLVM with
RTTI and exceptions turned on. The latest version of Alive2 is always intended
RTTI and exceptions turned on. The latest version Of Alive2 is always intended
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@@ -1,14 +1,17 @@
define void @src(ptr %0, i1 %1) {
%3 = alloca i64, align 8
call void @llvm.lifetime.start(i64 8, ptr %3)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these changes?

@glb = global ptr null
declare ptr @f(ptr %p)

declare void @llvm.lifetime.start(i64, ptr captures(none))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these changes?

Comment thread ir/memory.cpp
return { Byte(*this, ::raw_load(block.val, offset)), DataType(block.type) };
expr alive = isBlockAlive(expr::mkUInt(bid, Pointer::bitsShortBid()), local);
return { Byte(*this, mkIf_fold(alive, ::raw_load(block.val, offset),
Byte::mkPoisonByte(*this)())),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align

Comment thread ir/pointer.cpp
expr ret = p.isBlockAlive();
expr ret = expr::mkIf(p.isStackAllocated(),
(iswrite || is_asm) ? p.isBlockAlive() : true,
p.isBlockAlive());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify to (p.isStackAllocated() && !write) || p.isBlockAlive()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants