cgen: fix x86 segment-override memory operands in basic inline asm - #27782
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 499d0e3d31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !stmt.is_basic { | ||
| g.write('%') // escape percent with percent in extended assembly | ||
| } | ||
| g.write('%${arg.segment}:') |
There was a problem hiding this comment.
In asm goto blocks that only provide the label list (for example asm goto amd64 { jmp gs:[16]; ; ; ; done }), the parser still leaves stmt.is_basic true because it only counts outputs/inputs/clobbers, but asm_stmt emits __asm__ goto (... : : : : label), which is extended asm. This branch therefore writes %gs: instead of %%gs:, and GCC/Clang parse %g as an operand escape (operand number missing after %-letter). Please base this escaping on whether the emitted C asm is extended, including stmt.is_goto, rather than is_basic alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Codex provided feedback for a valid problem here.
There was a problem hiding this comment.
@medvednikov the checks failing on other code, likely due to the new parser.
…nds are escaped correctly
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Fixes #27777
In basic inline assembly (no operand lists), the template string is passed to the assembler verbatim, so segment overrides like
gs:[16]were emitted as%%gs:16and rejected withinvalid register name. Registers already handled this viastmt.is_basic; this applies the same escaping rule to segment prefixes: single%in basic asm,%%in extended asm.Verified with the issue's repro (
v -freestanding -cc clang -keepc .) — the binary now disassembles tomov %rdi,%gs:0x10. Added a coutput testdata pair covering both the basic and extended forms.