Skip to content

parser: fix invalid segment override on EQU FAR pointers#243

Open
alexvoste wants to merge 3 commits into
netwide-assembler:masterfrom
alexvoste:fix-far-pointer-equ
Open

parser: fix invalid segment override on EQU FAR pointers#243
alexvoste wants to merge 3 commits into
netwide-assembler:masterfrom
alexvoste:fix-far-pointer-equ

Conversation

@alexvoste

Copy link
Copy Markdown
Contributor

The parser incorrectly treated colons as memory segment overrides inside EQU directives because of an inverted far_jmp_ok check. This regression was introduced in commit 8981724. Removing the incorrect negation restores proper parsing of FAR pointer constants.

Fixes #242

@alexvoste alexvoste force-pushed the fix-far-pointer-equ branch from 539c0a4 to 4094093 Compare May 30, 2026 17:24
@hpax

hpax commented Jun 8, 2026

Copy link
Copy Markdown
Member

Rebased and committed.

@hpax hpax closed this Jun 8, 2026
@hpax

hpax commented Jun 8, 2026

Copy link
Copy Markdown
Member

This patch caused multiple tests to fail, because a segment override prefix was being dropped. Please make sure "make travis" patches before submitting a pull request.

I have reverted this patch, pending a proper fix.

@hpax hpax reopened this Jun 8, 2026
alexvoste added a commit to alexvoste/nasm that referenced this pull request Jun 9, 2026
@alexvoste alexvoste force-pushed the fix-far-pointer-equ branch from 4094093 to 73c202f Compare June 9, 2026 19:28
@alexvoste

Copy link
Copy Markdown
Contributor Author

@hpax I've addressed the segment override regression by refining the parser condition and ensured all local tests (including the failing obj and riprel tests) pass.

The CI checks are green. Please take another look.

test_img

@hpax

hpax commented Jun 11, 2026

Copy link
Copy Markdown
Member

Could you please add a proper commit message and Signed-off-by: line to your commit (see the SubmittingPatches file)?

I'm a bit confused about the asm/labels.c change; that looks like a separate memory leak fix? That is of course entirely legitimate (and appreciated), but it should be a separate patch with a separate patch description.

I really do appreciate your effort on this.

Signed-off-by: alexvoste <meshoff@proton.me>
Signed-off-by: alexvoste <meshoff@proton.me>
@alexvoste alexvoste force-pushed the fix-far-pointer-equ branch from 73c202f to 4842fa9 Compare June 12, 2026 08:10
@hpax

hpax commented Jun 14, 2026

Copy link
Copy Markdown
Member

The SoB line is great, but a commit description is really valuable when trying to figure out 20 years from now what is broken when something needed to change.

A description of the problem and why the fix is the correct one would be really appreciated.

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.

parsing or FAR pointers broken

2 participants