fix: guard group<=0 in ConvolutionDepthWise(::1D)::load_param to avoid FPE divide-by-zero (#6911) - #6919
Closed
FluxMind-O wants to merge 1 commit into
Closed
Conversation
…d FPE divide-by-zero When group == 0, the existing guard num_output % group != 0 itself performs integer division by zero, triggering SIGFPE before the rejection path is reached (issue Tencent#6911). The Vulkan int8 path already checked group == 0; the CPU and 1D load_param variants did not. Add a group <= 0 short-circuit before the modulo in both affected implementations, returning -100 (the existing reject-invalid-group code) without performing the division. group < 0 is rejected too (defensive). Normal-path behavior is unchanged. - src/layer/convolutiondepthwise.cpp: CPU ConvolutionDepthWise::load_param - src/layer/convolutiondepthwise1d.cpp: ConvolutionDepthWise1D::load_param Regression tests: - tests/test_convolutiondepthwise_invalid.cpp - tests/test_convolutiondepthwise1d_invalid.cpp assert load_param returns -100 for group = 0, -1, -8. Verified: before fix the new test aborts with SIGFPE (exit 136); after fix both new tests pass and existing test_convolutiondepthwise(_1/_oom) and test_convolutiondepthwise1d still pass.
Member
|
|
Author
|
Closing as duplicate — my apologies to the maintainers. After opening this I found that #6912 by @beilzx already fixes #6911 (same Thanks @beilzx for the prompt fix. I'll go review #6912 instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6911 — FPE divide-by-zero in
ConvolutionDepthWise::load_paramwhen a malformed param file providesgroup == 0.Problem
When
group == 0, the existing guardnum_output % group != 0itself performs an integer division by zero, triggering SIGFPE (FPE divide-by-zero) before the rejection path is reached. The CPU and 1D variants ofConvolutionDepthWise::load_paramwere missing thegroup <= 0check that the Vulkan int8 path already had (convolutiondepthwise_vulkan.cpp:597).Fix
Add a
group <= 0short-circuit guard before the modulo in both affectedload_paramimplementations, returning-100(the existing "reject invalid group" code) without performing the division:src/layer/convolutiondepthwise.cpp— CPUConvolutionDepthWise::load_paramsrc/layer/convolutiondepthwise1d.cpp—ConvolutionDepthWise1D::load_paramThe Vulkan variant calls the CPU
load_param, so it is covered transitively.group < 0is rejected too (defensive), matching the spirit of the existing guard. Normal-path behavior is unchanged: validgroup > 0withnum_output % group == 0still returns 0.Regression tests
tests/test_convolutiondepthwise_invalid.cpp— assertsload_paramreturns-100forgroup= 0, -1, -8.tests/test_convolutiondepthwise1d_invalid.cpp— same for the 1D variant.Verification
test_convolutiondepthwise_invalidaborts withFloating point exception (core dumped)(exit 136 / SIGFPE), reproducing bug: FPE (divide-by-zero) in ConvolutionDepthWise::load_param (src/layer/convolutiondepthwise.cpp:46) #6911.test_convolutiondepthwise,test_convolutiondepthwise_1,test_convolutiondepthwise_oom,test_convolutiondepthwise1dall still pass — no regression on the normal path.Checklist