Skip to content

Fix crashes in mgd77magref with a numeric -D - #9270

Merged
Esteban82 merged 4 commits into
masterfrom
fix-mgd77magref-dst
Oct 19, 2026
Merged

Esteban82 merged 4 commits into
masterfrom
fix-mgd77magref-dst

Conversation

@Esteban82

Copy link
Copy Markdown
Member

Claude analyzed mgd77magref and found a few bugs around the Dst buffer that lead to crashes. This PR proposes the fix. The scripts below show them (on current master the first three segfault; the last one segfaults after the error for the bad segment).

  1. -D<number> segfaults: parse() wrote the value into CM4_D.dst before it was allocated (it is allocated later in the module body).
  2. -D-8 behaved as -D8: the leading - was skipped before atof.
  3. With several input records a numeric -D read past the end of its 1-element dst array.
  4. A segment outside the Dst time span freed dst in MGD77_cm4field, so the next single-record segment wrote into a NULL pointer.
    dst is now allocated at parse time and freed only in Free_Ctrl. Adds test/mgd77/magref_dst.sh.

Tested with:

export GMT_END_SHOW=off
echo "-30 45 0 2000.08700533" | gmt mgd77magref -A+y -Fxyz/3 -D8
echo "-30 45 0 2000.08700533" | gmt mgd77magref -A+y -Fxyz/3 -D-8
printf -- "-30 45 0 2000.087\n-30 45 0 2000.5\n-30 45 0 2001.2\n" | gmt mgd77magref -A+y -Fxyz/3 -D8
printf -- "> bad\n-30 45 0 1900.5\n-30 45 0 1900.6\n> good\n-30 45 0 2000.5\n" | gmt mgd77magref -A+y -Fxyz/3

Written with Claude Sonnet 5.5, reviewed with Claude Opus 5.5.

This is the output with this fix:

-8.91517189718  3.78735619674   10.5974519577
-15.5064972641  4.44507960625   19.2142882413
-8.88607779405  3.77783244797   10.6115537859
-6.01975730977  2.8274257026    17.2528952866
-8.80171128582  2.58243039681   12.9876225555
> bad
INTDST -- Error: T (15202; 15203) LIES OUTSIDE OF DST TABLE TIME SPAN [35839; 60675] -- ABORT
mgd77magref [ERROR]: this segment has a record generating an error.
Unfortunately, this means all other eventually good
records are also ignored. Fix the bad record and rerun the command.
> good
-3.84802225651  2.23032262522   14.4369055462

Esteban82 and others added 4 commits October 16, 2026 17:26
…d77magref

parse() stored the Dst value in a buffer that was only allocated later, and
skipped the leading minus sign when converting. With several input records a
numeric -D also read past the one-element dst array.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
MGD77_cm4field freed it on that error, so the next single-record segment
wrote into a NULL pointer. Free_Ctrl now owns the buffer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Esteban82
Esteban82 requested review from joa-quim and seisman October 17, 2026 19:43
@Esteban82 Esteban82 added bug Something isn't working add-changelog Add PR to the changelog labels Oct 17, 2026
@Esteban82
Esteban82 merged commit 6e070b2 into master Oct 19, 2026
19 of 20 checks passed
@Esteban82
Esteban82 deleted the fix-mgd77magref-dst branch October 19, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add-changelog Add PR to the changelog bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants