Remove unreachable return statements after avm_internal_error calls. #5016
Remove unreachable return statements after avm_internal_error calls. #5016urvangjoshi wants to merge 3 commits into
Conversation
| AV2_COMMON *cm = &ctx->cpi->common; | ||
| avm_internal_error(&cm->error, AVM_CODEC_INVALID_PARAM, | ||
| "Incorrect buffer dimensions"); | ||
| return cm->error.error_code; |
There was a problem hiding this comment.
avm_internal_error() calls longjmp() only when setjmp() has been called. That is not the case here.
We should also verify setjmp() has been called in the other changes in this PR.
There was a problem hiding this comment.
Good point. There seem to be a few places in this file and also av2_dx_iface.c where setjmp has not been called.
Are you suggesting to add setjmp to those places?
Note that the clang is reporting these error likely because of how avm_internal_error() function is declared:
#define CLANG_ANALYZER_NORETURN
#if defined(__has_feature)
#if __has_feature(attribute_analyzer_noreturn)
#undef CLANG_ANALYZER_NORETURN
#define CLANG_ANALYZER_NORETURN __attribute__((analyzer_noreturn))
#endif
#endif
void avm_internal_error(struct avm_internal_error_info *info,
....
There was a problem hiding this comment.
In av2_cx_iface.c and av2_dx_iface.c we can delete the avm_internal_error() call and return the error code directly.
There was a problem hiding this comment.
That makes sense. I checked that most of the code in these files doesn't use avm_internal_error, so keeping only return statements instead is also consistent.
PTAL.
urvangjoshi
force-pushed
the
clang_build_fix
branch
2 times, most recently
from
ade92da to
16265dc
Compare
This fixes following type of build errors on LLVM 22.1.6: ``` error: 'return' will never be executed [-Werror,-Wunreachable-code-return] ```
urvangjoshi
force-pushed
the
clang_build_fix
branch
from
16265dc to
5e442d1
Compare
| @@ -374,13 +374,11 @@ void av2_apply_ccso_filter_for_row(AV2_COMMON *cm, MACROBLOCKD *xd, | |||
| avm_internal_error( | |||
| &cm->error, AVM_CODEC_ERROR, | |||
| "Invalid BRU activity in CCSO: only active SB can be filtered"); | |||
There was a problem hiding this comment.
The av2_apply_ccso_filter_for_row() function may be called transitively by ccso_row_worker(), which does not call setjmp().
| @@ -1972,7 +1972,6 @@ void av2_build_one_bawp_inter_predictor( | |||
| avm_internal_error( | |||
| (struct avm_internal_error_info *)&cm->error, AVM_CODEC_ERROR, | |||
| "Inter BAWP template cannot outside the valid reference range"); | |||
There was a problem hiding this comment.
av2_build_one_bawp_inter_predictor() may be called transitively by enc_row_mt_worker_hook(), which does not call setjmp().
Note that libaom handles this correctly. We can port libaom's changes to the AVM.
| AV2_COMMON *cm = &ctx->cpi->common; | ||
| avm_internal_error(&cm->error, AVM_CODEC_INVALID_PARAM, | ||
| "Incorrect buffer dimensions"); | ||
| return cm->error.error_code; |
There was a problem hiding this comment.
I suggest we move the changes to av2/av2_cx_iface.c and av2/av2_dx_iface.c to a separate PR so that they can be reviewed and merged first.
There was a problem hiding this comment.
Good idea! Moved to this PR: #5028
Marking this PR as Draft as I check about setjmp for other places.
3 participants
This fixes following type of build errors on LLVM 22.1.6:
Related: Ensure that setjmp is called before avm_internal_error in FilmGrainTableIOTest.
Closes #5002