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 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
FrankYFTang · Sep 25, 2024 · 9e89648 · 9e89648
1 parent 3606d75
commit 9e89648
Show file tree

Hide file tree

Showing 21 changed files with 102 additions and 62 deletions.
diff --git a/.github/workflows/icu4c.yml b/.github/workflows/icu4c.yml
@@ -67,6 +67,8 @@ jobs:
 
       - name: ICU4C with gcc
         env:
+          CXXFLAGS: -Wextra -Werror -Wno-return-local-addr
+          CFLAGS: -Werror
           PREFIX: /tmp/icu-prefix
         run: |
           mkdir build;
@@ -915,4 +917,4 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: icuexportdata_output
-          path: icuexportdata_tag-goes-here.zip
+          path: icuexportdata_tag-goes-here.zip
diff --git a/icu4c/source/common/ucnvmbcs.cpp b/icu4c/source/common/ucnvmbcs.cpp
@@ -3146,11 +3146,8 @@ ucnv_MBCSGetNextUChar(UConverterToUnicodeArgs *pArgs,
     if(c<0) {
         if(U_SUCCESS(*pErrorCode) && source==sourceLimit && lastSource<source) {
             /* incomplete character byte sequence */
-            uint8_t *bytes=cnv->toUBytes;
             cnv->toULength = static_cast<int8_t>(source - lastSource);
-            do {
-                *bytes++=*lastSource++;
-            } while(lastSource<source);
+            uprv_memcpy(cnv->toUBytes, lastSource, cnv->toULength);
             *pErrorCode=U_TRUNCATED_CHAR_FOUND;
         } else if(U_FAILURE(*pErrorCode)) {
             /* callback(illegal) */

diff --git a/icu4c/source/common/ucurr.cpp b/icu4c/source/common/ucurr.cpp
@@ -372,12 +372,8 @@ struct CReg : public icu::UMemory {
     CReg(const char16_t* _iso, const char* _id)
         : next(nullptr)
     {
-        int32_t len = static_cast<int32_t>(uprv_strlen(_id));
-        if (len > static_cast<int32_t>(sizeof(id) - 1)) {
-            len = (sizeof(id)-1);
-        }
-        uprv_strncpy(id, _id, len);
-        id[len] = 0;
+        uprv_strncpy(id, _id, sizeof(id)-1);
+        id[sizeof(id)-1] = 0;
         u_memcpy(iso, _iso, ISO_CURRENCY_CODE_LENGTH);
         iso[ISO_CURRENCY_CODE_LENGTH] = 0;
     }

diff --git a/icu4c/source/common/ushape.cpp b/icu4c/source/common/ushape.cpp
@@ -28,6 +28,7 @@
 #include "ubidi_props.h"
 #include "uassert.h"
 
+#include <limits>
 /*
  * This implementation is designed for 16-bit Unicode strings.
  * The main assumption is that the Arabic characters and their
@@ -747,6 +748,10 @@ handleGeneratedSpaces(char16_t *dest, int32_t sourceLength,
         }
     }
 
+    if (static_cast<size_t>(sourceLength) + 1 > std::numeric_limits<size_t>::max() / U_SIZEOF_UCHAR) {
+        *pErrorCode = U_INDEX_OUTOFBOUNDS_ERROR;
+        return 0;
+    }
     tempbuffer = static_cast<char16_t*>(uprv_malloc((sourceLength + 1) * U_SIZEOF_UCHAR));
     /* Test for nullptr */
     if(tempbuffer == nullptr) {

diff --git a/icu4c/source/i18n/calendar.cpp b/icu4c/source/i18n/calendar.cpp
@@ -828,9 +828,9 @@ Calendar::operator=(const Calendar &right)
         fWeekendCease            = right.fWeekendCease;
         fWeekendCeaseMillis      = right.fWeekendCeaseMillis;
         fNextStamp               = right.fNextStamp;
-        uprv_strncpy(validLocale, right.validLocale, sizeof(validLocale));
-        uprv_strncpy(actualLocale, right.actualLocale, sizeof(actualLocale));
+        uprv_strncpy(validLocale, right.validLocale, sizeof(validLocale)-1);
         validLocale[sizeof(validLocale)-1] = 0;
+        uprv_strncpy(actualLocale, right.actualLocale, sizeof(actualLocale)-1);
         actualLocale[sizeof(validLocale)-1] = 0;
     }
 

diff --git a/icu4c/source/i18n/formattedvalue.cpp b/icu4c/source/i18n/formattedvalue.cpp
@@ -193,6 +193,11 @@ ucfpos_close(UConstrainedFieldPosition* ptr) {
 }
 
 
+// -Wreturn-local-addr first found in https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Warning-Options.html#Warning-Options
+#if U_GCC_MAJOR_MINOR >= 409
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wreturn-local-addr"
+#endif
 U_CAPI const char16_t* U_EXPORT2
 ufmtval_getString(
         const UFormattedValue* ufmtval,
@@ -213,6 +218,9 @@ ufmtval_getString(
     // defined to return memory owned by the ufmtval argument.
     return readOnlyAlias.getBuffer();
 }
+#if U_GCC_MAJOR_MINOR >= 409
+#pragma GCC diagnostic pop
+#endif
 
 
 U_CAPI UBool U_EXPORT2

diff --git a/icu4c/source/i18n/number_rounding.cpp b/icu4c/source/i18n/number_rounding.cpp
@@ -278,23 +278,23 @@ Precision IncrementPrecision::withMinFraction(int32_t minFrac) const {
 }
 
 FractionPrecision Precision::constructFraction(int32_t minFrac, int32_t maxFrac) {
-    FractionSignificantSettings settings;
+    FractionSignificantSettings settings{};
     settings.fMinFrac = static_cast<digits_t>(minFrac);
     settings.fMaxFrac = static_cast<digits_t>(maxFrac);
     settings.fMinSig = -1;
     settings.fMaxSig = -1;
-    PrecisionUnion union_;
+    PrecisionUnion union_{};
     union_.fracSig = settings;
     return {RND_FRACTION, union_};
 }
 
 Precision Precision::constructSignificant(int32_t minSig, int32_t maxSig) {
-    FractionSignificantSettings settings;
+    FractionSignificantSettings settings{};
     settings.fMinFrac = -1;
     settings.fMaxFrac = -1;
     settings.fMinSig = static_cast<digits_t>(minSig);
     settings.fMaxSig = static_cast<digits_t>(maxSig);
-    PrecisionUnion union_;
+    PrecisionUnion union_{};
     union_.fracSig = settings;
     return {RND_SIGNIFICANT, union_};
 }
@@ -311,20 +311,20 @@ Precision::constructFractionSignificant(
     settings.fMaxSig = static_cast<digits_t>(maxSig);
     settings.fPriority = priority;
     settings.fRetain = retain;
-    PrecisionUnion union_;
+    PrecisionUnion union_{};
     union_.fracSig = settings;
     return {RND_FRACTION_SIGNIFICANT, union_};
 }
 
 IncrementPrecision Precision::constructIncrement(uint64_t increment, digits_t magnitude) {
-    IncrementSettings settings;
+    IncrementSettings settings{};
     // Note: For number formatting, fIncrement is used for RND_INCREMENT but not
     // RND_INCREMENT_ONE or RND_INCREMENT_FIVE. However, fIncrement is used in all
     // three when constructing a skeleton.
     settings.fIncrement = increment;
     settings.fIncrementMagnitude = magnitude;
     settings.fMinFrac = magnitude > 0 ? 0 : -magnitude;
-    PrecisionUnion union_;
+    PrecisionUnion union_{};
     union_.increment = settings;
     if (increment == 1) {
         // NOTE: In C++, we must return the correct value type with the correct union.
@@ -339,7 +339,7 @@ IncrementPrecision Precision::constructIncrement(uint64_t increment, digits_t ma
 }
 
 CurrencyPrecision Precision::constructCurrency(UCurrencyUsage usage) {
-    PrecisionUnion union_;
+    PrecisionUnion union_{};
     union_.currencyUsage = usage;
     return {RND_CURRENCY, union_};
 }

diff --git a/icu4c/source/i18n/number_skeletons.cpp b/icu4c/source/i18n/number_skeletons.cpp
@@ -1015,6 +1015,12 @@ blueprint_helpers::parseExponentSignOption(const StringSegment& segment, MacroPr
     return true;
 }
 
+// The function is called by skeleton::parseOption which called by skeleton::parseSkeleton
+// the data pointed in the return macros.unit is stack allocated in the parseSkeleton function.
+#if U_GCC_MAJOR_MINOR >= 1204
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-pointer"
+#endif
 void blueprint_helpers::parseCurrencyOption(const StringSegment& segment, MacroProps& macros,
                                             UErrorCode& status) {
     // Unlike ICU4J, have to check length manually because ICU4C CurrencyUnit does not check it for us
@@ -1034,6 +1040,9 @@ void blueprint_helpers::parseCurrencyOption(const StringSegment& segment, MacroP
     // Slicing is OK
     macros.unit = currency; // NOLINT
 }
+#if U_GCC_MAJOR_MINOR >= 1204
+#pragma GCC diagnostic pop
+#endif
 
 void
 blueprint_helpers::generateCurrencyOption(const CurrencyUnit& currency, UnicodeString& sb, UErrorCode&) {

diff --git a/icu4c/source/test/cintltst/cbiditst.c b/icu4c/source/test/cintltst/cbiditst.c
@@ -4730,6 +4730,7 @@ checkMaps(UBiDi *pBiDi, int32_t stringIndex, const char *src, const char *dest,
                 );
         testOK = false;
     }
+    memset(getIndexMap, 0, sizeof(getIndexMap));
     for (i = 0; i < srcLen; i++) {
         idx = ubidi_getVisualIndex(pBiDi, i, &rc);
         assertSuccessful("ubidi_getVisualIndex", &rc);

diff --git a/icu4c/source/test/cintltst/custrtrn.c b/icu4c/source/test/cintltst/custrtrn.c
@@ -1527,7 +1527,7 @@ static void Test_strToJavaModifiedUTF8(void) {
         0xee, 0x80, 0x81, 0xee, 0x80, 0x82, 0xee, 0x80, 0x83,
         0xed, 0xa0, 0x80, 0xed, 0xb0, 0x80, 0xed, 0xb0, 0x80, 0xed, 0xa0, 0x80, 0xc0, 0x80,
         0xed, 0xaf, 0xbf, 0xed, 0xbf, 0xbf,
-        0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0xc3, 0xad, 0xe0, 0xb8, 0x8e, 0x6f
+        0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0xc3, 0xad, 0xe0, 0xb8, 0x8e, 0x6f, 0
     };
     static const UChar shortSrc[]={
         0xe01, 0xe1, 0x61
@@ -1554,7 +1554,7 @@ static void Test_strToJavaModifiedUTF8(void) {
     p=u_strToJavaModifiedUTF8(dest, (int32_t)sizeof(dest), &length,
                               src, UPRV_LENGTHOF(src), &errorCode);
     if( U_FAILURE(errorCode) || p!=dest ||
-        length!=UPRV_LENGTHOF(expected) || 0!=memcmp(dest, expected, length) ||
+        length!=(UPRV_LENGTHOF(expected)-1) || 0!=memcmp(dest, expected, length) ||
         dest[length]!=0
     ) {
         log_err("u_strToJavaModifiedUTF8(normal) failed - %s\n", u_errorName(errorCode));
@@ -1565,18 +1565,18 @@ static void Test_strToJavaModifiedUTF8(void) {
     p=u_strToJavaModifiedUTF8(dest, (int32_t)sizeof(dest), NULL,
                               src, UPRV_LENGTHOF(src), &errorCode);
     if( U_FAILURE(errorCode) || p!=dest ||
-        0!=memcmp(dest, expected, UPRV_LENGTHOF(expected)) ||
-        dest[UPRV_LENGTHOF(expected)]!=0
+        0!=memcmp(dest, expected, (UPRV_LENGTHOF(expected)-1)) ||
+        dest[(UPRV_LENGTHOF(expected)-1)]!=0
     ) {
         log_err("u_strToJavaModifiedUTF8(normal, pLength=NULL) failed - %s\n", u_errorName(errorCode));
     }
     memset(dest, 0xff, sizeof(dest));
     errorCode=U_ZERO_ERROR;
     length=-5;
-    p=u_strToJavaModifiedUTF8(dest, UPRV_LENGTHOF(expected), &length,
+    p=u_strToJavaModifiedUTF8(dest, (UPRV_LENGTHOF(expected)-1), &length,
                               src, UPRV_LENGTHOF(src), &errorCode);
     if( errorCode!=U_STRING_NOT_TERMINATED_WARNING || p!=dest ||
-        length!=UPRV_LENGTHOF(expected) || 0!=memcmp(dest, expected, length) ||
+        length!=(UPRV_LENGTHOF(expected)-1) || 0!=memcmp(dest, expected, length) ||
         dest[length]!=(char)0xff
     ) {
         log_err("u_strToJavaModifiedUTF8(tight) failed - %s\n", u_errorName(errorCode));
@@ -1604,10 +1604,10 @@ static void Test_strToJavaModifiedUTF8(void) {
     memset(dest, 0xff, sizeof(dest));
     errorCode=U_ZERO_ERROR;
     length=-5;
-    p=u_strToJavaModifiedUTF8(dest, UPRV_LENGTHOF(expected)/2, &length,
+    p=u_strToJavaModifiedUTF8(dest, (UPRV_LENGTHOF(expected)-1)/2, &length,
                               src, UPRV_LENGTHOF(src), &errorCode);
     if( errorCode!=U_BUFFER_OVERFLOW_ERROR ||
-        length!=UPRV_LENGTHOF(expected) || dest[UPRV_LENGTHOF(expected)/2]!=(char)0xff
+        length!=(UPRV_LENGTHOF(expected)-1) || dest[(UPRV_LENGTHOF(expected)-1)/2]!=(char)0xff
     ) {
         log_err("u_strToJavaModifiedUTF8(overflow) failed - %s\n", u_errorName(errorCode));
     }
@@ -1617,7 +1617,7 @@ static void Test_strToJavaModifiedUTF8(void) {
     p=u_strToJavaModifiedUTF8(NULL, 0, &length,
                               src, UPRV_LENGTHOF(src), &errorCode);
     if( errorCode!=U_BUFFER_OVERFLOW_ERROR ||
-        length!=UPRV_LENGTHOF(expected) || dest[0]!=(char)0xff
+        length!=(UPRV_LENGTHOF(expected)-1) || dest[0]!=(char)0xff
     ) {
         log_err("u_strToJavaModifiedUTF8(pure preflighting) failed - %s\n", u_errorName(errorCode));
     }

diff --git a/icu4c/source/test/cintltst/hpmufn.c b/icu4c/source/test/cintltst/hpmufn.c
@@ -116,7 +116,7 @@ static void * U_CALLCONV myMemRealloc(const void *context, void *mem, size_t siz
     }
     retPtr = realloc(p, size+sizeof(ctest_AlignedMemory));
     if (retPtr != NULL) {
-        p += sizeof(ctest_AlignedMemory);
+        retPtr += sizeof(ctest_AlignedMemory);
     }
     return retPtr;
 }

diff --git a/icu4c/source/test/cintltst/ucptrietest.c b/icu4c/source/test/cintltst/ucptrietest.c
@@ -744,7 +744,13 @@ trieTestGolden(const char *testName,
         goto cleanup;
     }
     fseek(stream, 0, SEEK_SET);
-    fread(memoryBuffer, 1, fsize, stream);
+    long rsize = fread(memoryBuffer, 1, fsize, stream);
+    if (rsize != fsize) {
+        log_err(
+            "Golden files for '%s' fread %d bytes fail. Only get %d",
+            testName, fsize, rsize);
+    }
+
 
     int32_t testResult = uprv_compareGoldenFiles(
         memoryBuffer, fsize,

diff --git a/icu4c/source/test/cintltst/uformattedvaluetst.c b/icu4c/source/test/cintltst/uformattedvaluetst.c
@@ -144,13 +144,13 @@ static void AssertAllPartsEqual(
     UErrorCode status = U_ZERO_ERROR;
 
     char message[256];
-    uprv_strncpy(message, messagePrefix, 256);
+    uprv_strncpy(message, messagePrefix, UPRV_LENGTHOF(message)-2);
     int32_t prefixEnd = (int32_t)uprv_strlen(messagePrefix);
     message[prefixEnd++] = ':';
     message[prefixEnd++] = ' ';
-    U_ASSERT(prefixEnd < 256);
+    U_ASSERT(prefixEnd < UPRV_LENGTHOF(message));
 
-#define AAPE_MSG(suffix) (uprv_strncpy(message+prefixEnd, suffix, 256-prefixEnd)-prefixEnd)
+#define AAPE_MSG(suffix) (uprv_strncpy(message+prefixEnd, suffix, UPRV_LENGTHOF(message)-prefixEnd)-prefixEnd)
 
     UFieldCategory _category = ucfpos_getCategory(ucfpos, &status);
     assertSuccess(AAPE_MSG("_"), &status);

diff --git a/icu4c/source/test/cintltst/utf16tst.c b/icu4c/source/test/cintltst/utf16tst.c
@@ -397,6 +397,10 @@ static void TestNextPrevChar(void){
 
 }
 
+#if U_GCC_MAJOR_MINOR >= 406
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
 /* keep this in sync with utf8tst.c's TestNulTerminated() */
 static void TestNulTerminated(void) {
     static const UChar input[]={
@@ -477,6 +481,9 @@ static void TestNulTerminated(void) {
         } while(++j<i);
     } while(c!=0);
 }
+#if U_GCC_MAJOR_MINOR >= 406
+#pragma GCC diagnostic pop
+#endif
 
 static void TestFwdBack(void){
     static UChar input[]={0x0061, 0xd800, 0xdc00, 0xdbff, 0xdfff, 0x0062, 0xd841, 0xd7ff, 0xd841, 0xdc41, 0xdc00, 0x0000};

diff --git a/icu4c/source/test/cintltst/utf8tst.c b/icu4c/source/test/cintltst/utf8tst.c
@@ -449,6 +449,10 @@ static void TestNextPrevChar(void) {
     }
 }
 
+#if U_GCC_MAJOR_MINOR >= 406
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
 /* keep this in sync with utf16tst.c's TestNulTerminated() */
 static void TestNulTerminated(void) {
     static const uint8_t input[]={
@@ -543,6 +547,9 @@ static void TestNulTerminated(void) {
         } while(++j<i);
     } while(c!=0);
 }
+#if U_GCC_MAJOR_MINOR >= 406
+#pragma GCC diagnostic pop
+#endif
 
 static void TestNextPrevNonCharacters(void) {
     /* test non-characters */

diff --git a/icu4c/source/test/intltest/rbbitst.cpp b/icu4c/source/test/intltest/rbbitst.cpp
@@ -4584,7 +4584,7 @@ void RBBITest::RunMonkey(BreakIterator *bi, RBBIMonkeyKind &mk, const char *name
                     UErrorCode status = U_ZERO_ERROR;
                     u_charName(c, U_EXTENDED_CHAR_NAME, cName, sizeof(cName), &status);
 
-                    char buffer[200];
+                    char buffer[280];
                     auto ret = snprintf(buffer, sizeof(buffer),
                              "%4s %3i :  %1s  %1s  %10s  %-*s  %-40s  %-40s",
                              currentLineFlag.c_str(),

diff --git a/icu4c/source/test/intltest/restsnew.cpp b/icu4c/source/test/intltest/restsnew.cpp
@@ -1489,7 +1489,7 @@ void NewResourceBundleTest::TestTrace() {
 
     assertEquals("Start position stability coverage", 0x3000, UTRACE_UDATA_START);
 
-    const void* context;
+    const void* context = nullptr;
     utrace_setFunctions(context, nullptr, nullptr, traceData);
     utrace_setLevel(UTRACE_VERBOSE);
 

diff --git a/icu4c/source/test/iotest/stream.cpp b/icu4c/source/test/iotest/stream.cpp
@@ -76,7 +76,7 @@ static void U_CALLCONV TestStream()
         return;
     }
     ucnv_close(defConv);
-    strncpy(defConvName, ucnv_getDefaultName(), UPRV_LENGTHOF(defConvName));
+    strncpy(defConvName, ucnv_getDefaultName(), UPRV_LENGTHOF(defConvName)-1);
     ucnv_setDefaultName("UTF-8");
 
     static const char * const TESTSTRING = "\x20\x74\x48\x69\x73\xCE\xBC\xE2\x80\x82\x20\x6D\x75\x20\x77\x6F\x72\x6C\x64";

diff --git a/icu4c/source/tools/ctestfw/ctest.c b/icu4c/source/tools/ctestfw/ctest.c
@@ -181,7 +181,9 @@ static TestNode *createTestNode(const char* name, int32_t nameLen)
     newNode->sibling = NULL;
     newNode->child = NULL;
 
-    strncpy( newNode->name, name, nameLen );
+    if (nameLen > 0) {
+        strncpy( newNode->name, name, nameLen );
+    }
     newNode->name[nameLen] = 0;
 
     return  newNode;

diff --git a/icu4c/source/tools/icuexportdata/icuexportdata.cpp b/icu4c/source/tools/icuexportdata/icuexportdata.cpp
@@ -331,13 +331,13 @@ void dumpGeneralCategoryMask(FILE* f) {
 
 
     fprintf(f, "mask_for = \"General_Category\"\n");
-    uint32_t minValue = u_getIntPropertyMinValue(UCHAR_GENERAL_CATEGORY);
+    int32_t minValue = u_getIntPropertyMinValue(UCHAR_GENERAL_CATEGORY);
     U_ASSERT(minValue >= 0);
-    uint32_t maxValue = u_getIntPropertyMaxValue(UCHAR_GENERAL_CATEGORY);
+    int32_t maxValue = u_getIntPropertyMaxValue(UCHAR_GENERAL_CATEGORY);
     U_ASSERT(maxValue >= 0);
 
     fprintf(f, "values = [\n");
-    for (uint32_t v = minValue; v <= maxValue; v++) {
+    for (int32_t v = minValue; v <= maxValue; v++) {
         dumpValueEntry(uproperty, U_MASK(v), true, f);
 
         // We want to dump these masks "in order", which means they