Skip to content

ggml : fix for non-virtual destructor issue of gguf_writer_base#25867

Merged
JohannesGaessler merged 1 commit into
ggml-org:masterfrom
yymin1022:master
Jul 25, 2026
Merged

ggml : fix for non-virtual destructor issue of gguf_writer_base#25867
JohannesGaessler merged 1 commit into
ggml-org:masterfrom
yymin1022:master

Conversation

@yymin1022

Copy link
Copy Markdown
Contributor

Overview

gguf_writer_base declares virtual member functions but its destructor
was non-virtual. Deleting a derived writer instance through a
gguf_writer_base* (or any base-class pointer/reference) skips the
derived class's destructor, because it is not virtual.

This can leak resources owned by the derived writer and is undefined behavior per the C++
standard when the dynamic type differs from the static type at the
point of deletion.

Change

Add virtual to gguf_writer_base's destructor in ggml/src/gguf.cpp.

-    ~gguf_writer_base(void) = default;
+    virtual ~gguf_writer_base(void) = default;

Additional information

Found while cross-compiling ggml/llama.cpp as a static library inside
an AOSP tree (vendor partition), which enables
-Werror=non-virtual-dtor unconditionally by AOSP build rule.

Confirmed the class has no other polymorphic-deletion issues after the fix; build completes
cleanly with this flag enabled.

No functional change to existing single-inheritance, base-pointer-free
usage; this only affects correctness when a derived writer is deleted
via a base pointer, which is the intended usage pattern given the
virtual methods already present on the class.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES (Claude Sonnet5, Just debug for my AOSP build issue, and this PR is fuilly written by myself)

…gguf_writer_base

Without a virtual destructor, deleting a derived object through a
base-class pointer only invokes the base destructor, skipping the
derived one.
@github-actions github-actions Bot added the ggml changes relating to the ggml tensor library for machine learning label Jul 18, 2026
@yymin1022 yymin1022 changed the title Update ggml/src/gguf.cpp : Defined virtual keyword for destructor of … ggml : fix for non-virtual destructor issue of gguf_writer_base Jul 20, 2026
@yymin1022

Copy link
Copy Markdown
Contributor Author

@JohannesGaessler Hello, could you please take a look when you get a chance?

@JohannesGaessler

Copy link
Copy Markdown
Contributor

I've given this PR priority since you went so far as to email me about it but this was absolutely not warranted on your end. The gguf_write_base class is only visible in gguf.cpp and it is never used as a pointer where this UB would surface. So this PR, while it avoids a potential bug in the future, is not at all urgent. Don't send me emails like this again or I will ban you.

@ggml-org/maintainers can I please get a second approval?

@JohannesGaessler
JohannesGaessler merged commit fb92d8f into ggml-org:master Jul 25, 2026
18 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants