Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICU-22767 Fix GCC warning and turn warning to errors #3129

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

FrankYFTang
Copy link
Contributor

@FrankYFTang FrankYFTang commented Sep 5, 2024

-Wdangling-pointer can first found in 12.4

Warning Options (Using the GNU Compiler Collection (GCC))

but not in 11.5

-Wstringop-overflow can first found in 11.5

Warning Options (Using the GNU Compiler Collection (GCC))

but not in 10.5

-Wreturn-local-addr first found in 4.8.5

Warning Options - Using the GNU Compiler Collection (GCC)

but not in 4.7.4

-Warray-bounds first found in 4.3.6

Warning Options - Using the GNU Compiler Collection (GCC)

but not in 4.2.4

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22767
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/common/ucurr.cpp is different
  • icu4c/source/i18n/decNumber.cpp is different
  • icu4c/source/i18n/formattedvalue.cpp is different
  • icu4c/source/i18n/number_skeletons.cpp is different
  • icu4c/source/test/cintltst/utf16tst.c is now changed in the branch
  • icu4c/source/test/cintltst/utf8tst.c is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/formattedvalue.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/decNumber.cpp is different
  • icu4c/source/i18n/formattedvalue.cpp is different
  • icu4c/source/i18n/number_skeletons.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@roubert
Copy link
Member

roubert commented Sep 5, 2024

The gcc-10-stdlib17 tests still fail with -Werror=return-local-addr.

@FrankYFTang FrankYFTang changed the title ICU-22716 Fix GCC warning and turn warning to errors ICU-22767 Fix GCC warning and turn warning to errors Sep 5, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

icu4c/source/common/ucnvmbcs.cpp Outdated Show resolved Hide resolved
icu4c/source/common/ucnvmbcs.cpp Outdated Show resolved Hide resolved
icu4c/source/common/ushape.cpp Outdated Show resolved Hide resolved
icu4c/source/test/cintltst/custrtrn.c Show resolved Hide resolved
@FrankYFTang
Copy link
Contributor Author

@markusicu
I have trouble to reproduce the following in my local machine and understand these messages. Do you have any clue how to fix it?

In file included from ../../../icu4c/source/tools/toolutil/ppucd.cpp:21:
In member function ‘icu_76::UniProps& icu_76::UniProps::operator=(const icu_76::UniProps&)’,
    inlined from ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’ at ../../../icu4c/source/tools/toolutil/ppucd.cpp:239:25:
../../../icu4c/source/tools/toolutil/ppucd.h:43:23: error: writing 16 bytes into a region of size 0 [-Werror=stringop-overflow=]
   43 | struct U_TOOLUTIL_API UniProps {
      |                       ^~~~~~~~
../../../icu4c/source/tools/toolutil/ppucd.h: In member function ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’:
../../../icu4c/source/tools/toolutil/ppucd.h:49:13: note: at offset 0 to object ‘icu_76::UniProps::start’ with size 4 declared here
   49 |     UChar32 start, end;
      |             ^~~~~
In member function ‘icu_76::UniProps& icu_76::UniProps::operator=(const icu_76::UniProps&)’,
    inlined from ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’ at ../../../icu4c/source/tools/toolutil/ppucd.cpp:235:25:
../../../icu4c/source/tools/toolutil/ppucd.h:43:23: error: writing 16 bytes into a region of size 0 [-Werror=stringop-overflow=]
   43 | struct U_TOOLUTIL_API UniProps {
      |                       ^~~~~~~~
../../../icu4c/source/tools/toolutil/ppucd.h: In member function ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’:
../../../icu4c/source/tools/toolutil/ppucd.h:49:13: note: at offset 0 to object ‘icu_76::UniProps::start’ with size 4 declared here
   49 |     UChar32 start, end;
      |             ^~~~~
In member function ‘icu_76::UniProps& icu_76::UniProps::operator=(const icu_76::UniProps&)’,
    inlined from ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’ at ../../../icu4c/source/tools/toolutil/ppucd.cpp:248:21:
../../../icu4c/source/tools/toolutil/ppucd.h:43:23: error: writing 16 bytes into a region of size 0 [-Werror=stringop-overflow=]
   43 | struct U_TOOLUTIL_API UniProps {
      |                       ^~~~~~~~
../../../icu4c/source/tools/toolutil/ppucd.h: In member function ‘const icu_76::UniProps* icu_76::PreparsedUCD::getProps(icu_76::UnicodeSet&, UErrorCode&)’:
../../../icu4c/source/tools/toolutil/ppucd.h:49:13: note: at offset 0 to object ‘icu_76::UniProps::start’ with size 4 declared here
   49 |     UChar32 start, end;
      |             ^~~~~
cc1plus: all warnings being treated as errors
*** Failed compilation command follows: ----------------------------------------------------------

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/icu4c.yml is different
  • icu4c/source/common/ucnvmbcs.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang added a commit to FrankYFTang/icu that referenced this pull request Sep 9, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/common/ucnvmbcs.cpp is different
  • icu4c/source/i18n/decNumber.cpp is different
  • icu4c/source/i18n/number_rounding.cpp is different
  • icu4c/source/i18n/number_skeletons.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/common/ucnvmbcs.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang
Copy link
Contributor Author

PTAL

.github/workflows/icu4c.yml Outdated Show resolved Hide resolved
.github/workflows/icu4c.yml Outdated Show resolved Hide resolved
Copy link
Member

@roubert roubert left a comment

Choose a reason for hiding this comment

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

With the warnings added to the gcc-debug-build-and-test job, you got yet one more maybe-uninitialized that'll need to be addressed.

icu4c/source/test/cintltst/uformattedvaluetst.c Outdated Show resolved Hide resolved
icu4c/source/common/ushape.cpp Outdated Show resolved Hide resolved
icu4c/source/i18n/decNumber.cpp Outdated Show resolved Hide resolved
icu4c/source/i18n/formattedvalue.cpp Outdated Show resolved Hide resolved
icu4c/source/i18n/formattedvalue.cpp Show resolved Hide resolved
icu4c/source/i18n/number_rounding.cpp Outdated Show resolved Hide resolved
icu4c/source/test/cintltst/utf16tst.c Outdated Show resolved Hide resolved
icu4c/source/test/cintltst/utf8tst.c Outdated Show resolved Hide resolved
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang added a commit to FrankYFTang/icu that referenced this pull request Sep 25, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang added a commit to FrankYFTang/icu that referenced this pull request Sep 25, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang added a commit to FrankYFTang/icu that referenced this pull request Sep 25, 2024
ICU-22716 use pre-existing task

ICU-22716 Fix

ICU-22716 Fix unitialization

Update icu4c/source/common/ushape.cpp

Co-authored-by: Fredrik Roubert <[email protected]>

ICU-22716 change macro

ICU-22716 Add document about the macro

ICU-22716 Addres review feedback

ICU-22767 Fix GCC warning and turn warning to errors

ICU-22716 use pre-existing task

ICU-22716 Fix

ICU-22716 Fix unitialization

Update icu4c/source/common/ushape.cpp

Co-authored-by: Fredrik Roubert <[email protected]>

ICU-22716 change macro

ICU-22716 Add document about the macro

ICU-22716 Addres review feedback

ICU-22767 Fix GCC warning and turn warning to errors

ICU-22716 use pre-existing task

ICU-22716 Fix

ICU-22716 Fix unitialization

Update icu4c/source/common/ushape.cpp

Co-authored-by: Fredrik Roubert <[email protected]>

ICU-22767 Fix GCC warning and turn warning to errors

ICU-22716 use pre-existing task

ICU-22716 Fix

ICU-22716 Fix unitialization

Update icu4c/source/common/ushape.cpp

Co-authored-by: Fredrik Roubert <[email protected]>

ICU-22716 change macro

ICU-22716 Add document about the macro

ICU-22716 Addres review feedback

ICU-22767 Fix GCC warning and turn warning to errors

ICU-22716 use pre-existing task

ICU-22716 Fix

ICU-22716 Fix unitialization

Update icu4c/source/common/ushape.cpp

Co-authored-by: Fredrik Roubert <[email protected]>

ICU-22716 change macro

ICU-22716 Add document about the macro

ICU-22716 Address feedback

ICU-22767 Fix GCC warning and turn warning to errors

See unicode-org#3129
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang added a commit to FrankYFTang/icu that referenced this pull request Sep 25, 2024
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang
Copy link
Contributor Author

PTAL I removed unncessary changes

@@ -67,6 +67,8 @@ jobs:

- name: ICU4C with gcc
env:
CXXFLAGS: -Wextra -Werror -Wno-return-local-addr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
CXXFLAGS: -Wextra -Werror -Wno-return-local-addr
CXXFLAGS: -Wextra -Werror -Wno-error=return-local-addr

I recommend not hiding this warning entirely, but instead just not treat it as an error.

@FrankYFTang
Copy link
Contributor Author

During the ICUTC this morning. I agree to Markus we should hold this till 77.1

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.

4 participants