Skip to content

ci: add RelWithDebInfo build and fix free-nonheap-object error - #812

Merged
wgtmac merged 3 commits into
apache:mainfrom
zhjwpku:ci/relwithdebinfo-build
Jul 11, 2026
Merged

ci: add RelWithDebInfo build and fix free-nonheap-object error#812
wgtmac merged 3 commits into
apache:mainfrom
zhjwpku:ci/relwithdebinfo-build

Conversation

@zhjwpku

@zhjwpku zhjwpku commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Adding a RelWithDebInfo build revealed a build error. This PR fixes the issue.

error: ‘void operator delete(void*, std::size_t)’ called on a pointer to an unallocated object ‘1’ [-Werror=free-nonheap-object]

Comment thread src/iceberg/json_serde.cc
FromJsonMap<std::shared_ptr<SnapshotRef>>(json, kRefs, SnapshotRefFromJson));
ICEBERG_ASSIGN_OR_RAISE(auto refs, FromJsonMap<std::shared_ptr<SnapshotRef>>(
json, kRefs, SnapshotRefFromJson));
table_metadata->refs = std::move(refs);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seems like a more readable version of previous version, however it seems disconnected from the change discussed in this PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is related to the new RelWithDebInfo coverage: that build exposed the -Werror=free-nonheap-object failure, so this change avoids assigning directly through ICEBERG_ASSIGN_OR_RAISE and fixes the issue surfaced by the new CI job. I'll update the PR title to make that relationship clearer.

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.

This seems like a GCC bug?

name: AMD64 Ubuntu 26.04 (${{ matrix.cmake_build_type }})
runs-on: ubuntu-26.04
timeout-minutes: 30
timeout-minutes: 60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is my understanding correct that the timeout was doubled becuase now for ubuntu 2 build configurations are going to be executed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not really. Each build runs as its own job. I doubled that because this is a cold build, no cache can be utilized. Also, RelWithDebInfo may take longer since it's an -O2 build. It shouldn't take this long once it's merged into main and sccache starts taking effect.

@zhjwpku zhjwpku changed the title ci: add RelWithDebInfo build to trigger a compile error ci: add RelWithDebInfo build and fix free-nonheap-object error Jul 6, 2026
@wgtmac

wgtmac commented Jul 11, 2026

Copy link
Copy Markdown
Member

Thanks @zhjwpku for improving this and @evindj for the review!

@wgtmac
wgtmac merged commit aa36d35 into apache:main Jul 11, 2026
22 checks passed
@zhjwpku
zhjwpku deleted the ci/relwithdebinfo-build branch July 11, 2026 11:52
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.

3 participants