Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Do not run asserts for WASI alignment when not targeting WASI - #19925

Merged
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi
May 11, 2024
Merged

Do not run asserts for WASI alignment when not targeting WASI#19925
Vexu merged 4 commits into
ziglang:masterfrom
190n:avoid-wasi-asserts-on-non-wasi

Conversation

@190n

@190n190n commented May 10, 2024

Copy link
Copy Markdown
Contributor

Fixes#19665

It seems possible that the last two asserts should now be uncommented as well. I don't know enough about why they were commented in the first place.

@nektro

Copy link
Copy Markdown
Contributor

how is the file getting referenced in a non-wasi build?

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

I think this is at least one of the culprits:

zig/lib/std/std.zig

Lines 115 to 119 in e69caaa

pubconstOptions=struct {
enable_segfault_handler: bool=debug.default_enable_segfault_handler,
/// Function used to implement `std.fs.cwd` for WASI.
wasiCwd: fn () os.wasi.fd_t=fs.defaultWasiCwd,

I had a reproduction in #19919, although I closed that issue since I didn't realize it was a duplicate of #19665. The issue went away for me when I removed a call to std.log, which was indirectly using my Options struct.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Tests still pass locally with the i64/u64 alignment checks enabled, including x86-linux which seems to be the original reason they got disabled, so I'm pushing that change as well.

@nektro

Copy link
Copy Markdown
Contributor

I'd recommend changing std.options.wasiCwd to be a void on non-wasi rather than editing the wasi file. files named for a target shouldn't have to worry about not being on that target, the bug imo is the spillage causing that to be evaluated.

@190n190n changed the title Do not run asserts for WASI alignment when not targeting WASIMake std.Options.wasiCwd void on non-WASI targetsMay 10, 2024
@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Oh, that's failing on 32-bit x86 again. It must be referenced somewhere else too.

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

It seems like a few WASI-specific functions in std.posix get analyzed on all targets, and they reference WASI types in their signatures/returns. e.g. (targeting x86):

error: the following command failed with 1 compilation errors:
/home/ben/zig/build/stage4/bin/zig test -freference-trace=256 -ODebug -target x86-linux-gnu -mcpu baseline -Mroot=/home/ben/zig/lib/std/std.zig -lc --test-filter std-wasm32-wasi.0.1...0.1-musl-generic-Debug-libc --cache-dir /home/ben/zig/zig-cache --global-cache-dir /home/ben/.cache/zig --name test --zig-lib-dir /home/ben/zig/lib --listen=- test-std
└─ run test std-wasm32-wasi.0.1...0.1-musl-generic-Debug
└─ zig test Debug wasm32-wasi 1 errors
/home/ben/zig/lib/std/posix.zig:1659:5: error: found compile log statement
@compileLog("openatWasi");
^~~~~~~~~~~~~~~~~~~~~~~~~

Should we

  • make all these params conditional on builtin.os.tag (and I'm not sure if this is the only place causing such issues, as -freference-trace isn't being very helpful)?
  • move these all into a module that's only called on WASI?
  • switch back to checking the OS inside std/os/wasi.zig?
  • something else?

@Vexu

Vexu commented May 10, 2024

Copy link
Copy Markdown
Member
  • switch back to checking the OS inside std/os/wasi.zig?

Like I said in the issue #19665 (comment)

@190n

190n commented May 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Yeah, that seems probably easier at this point. I wanted to try @nektro's option but it seems like there are a lot more WASI references throughout std and it'll be hard to find them all.

@190n190n changed the title Make std.Options.wasiCwd void on non-WASI targetsDo not run asserts for WASI alignment when not targeting WASIMay 10, 2024
@Vexu
Vexu enabled auto-merge (squash) May 10, 2024 19:20
@Vexu
Vexu merged commit cc39ce2 into ziglang:masterMay 11, 2024
Sign up for freeto 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.

assert(@alignof(i16) == 2) fails on 8-bit AVR MCU

3 participants

@190n@nektro@Vexu