From 551fd165bef4413bd1bc9ce4fb8f0df7dbabf89e Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Wed, 16 Mar 2022 14:54:28 -0700 Subject: [PATCH 1/7] GCC 10.3: Wformat-overflow Spotted buffer overflows in `sprintf` with GCC 10.3 --- Dataset.cpp | 2 ++ PltApp.cpp | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/Dataset.cpp b/Dataset.cpp index b406325..1d1f9a3 100644 --- a/Dataset.cpp +++ b/Dataset.cpp @@ -431,12 +431,14 @@ void Dataset::DatasetRender(const Box &alignedRegion, AmrPicture *apptr, XmStringFree(sNewLevel); sprintf(minInfoV, fstring, rMin); + // warning: '%s' directive writing up to 159 bytes into a region of size 156 [-Wformat-overflow=] sprintf(minInfo, "Min:%s", minInfoV); XmString sNewMin = XmStringCreateSimple(minInfo); XtVaSetValues(wMinValue, XmNlabelString, sNewMin, NULL); XmStringFree(sNewMin); sprintf(maxInfoV, fstring, rMax); + // warning: '%s' directive writing up to 159 bytes into a region of size 156 [-Wformat-overflow=] sprintf(maxInfo, "Max:%s", maxInfoV); XmString sNewMax = XmStringCreateSimple(maxInfo); XtVaSetValues(wMaxValue, XmNlabelString, sNewMax, NULL); diff --git a/PltApp.cpp b/PltApp.cpp index 1b1cd4b..f1cdc7d 100644 --- a/PltApp.cpp +++ b/PltApp.cpp @@ -2832,6 +2832,7 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); + // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] sprintf(range, "Min: %s Max: %s", fMin, fMax); strcpy(saveRangeString, range); @@ -2964,9 +2965,11 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { pltAppState->CurrentDerivedNumber(), rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); + // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] sprintf(range, "Min: %s", fMin); XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); //XtVaSetValues(wid, XmNleftOffset, 20, NULL); + // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] sprintf(range, "Max: %s", fMax); XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); @@ -2975,8 +2978,10 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); + // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] sprintf(range, "Min: %s", fMin); XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); + // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] sprintf(range, "Max: %s", fMax); XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); From c3c74b4939bcc8f38837ffba2abb92cbeed65bc3 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 10:41:43 -0700 Subject: [PATCH 2/7] CI: Compile with `-Werror` --- .github/workflows/ubuntu.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 06eadc7..5b189fe 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -11,6 +11,8 @@ jobs: name: GCC DP 2D Profiling runs-on: ubuntu-20.04 if: github.event.pull_request.draft == false + env: + CXXFLAGS: "-Werror" steps: - uses: actions/checkout@v3 - uses: actions/checkout@v3 @@ -26,4 +28,5 @@ jobs: run: | make -j 2 \ AMREX_HOME=./amrex \ - USE_PROFPARSER=TRUE + USE_PROFPARSER=TRUE \ + EXTRACXXFLAGS="${CXXFLAGS}" From 5646b5ac1ebe1d810f505add4b0c346591e0beec Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 10:47:26 -0700 Subject: [PATCH 3/7] Dataset & PltApp: Fix Buffer Overflows Fix string buffer overflows in two files. --- Dataset.cpp | 15 ++++++++------- PltApp.cpp | 32 ++++++++++++++++---------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/Dataset.cpp b/Dataset.cpp index 1d1f9a3..69f7d73 100644 --- a/Dataset.cpp +++ b/Dataset.cpp @@ -29,6 +29,7 @@ const int MAXINDEXCHARS = 4; #include #include +#include #include #include using std::ostringstream; @@ -420,7 +421,7 @@ void Dataset::DatasetRender(const Box &alignedRegion, AmrPicture *apptr, largestWidth = std::max(8, largestWidth); // for no data string } - char levelInfo[Amrvis::LINELENGTH], maxInfo[Amrvis::LINELENGTH], minInfo[Amrvis::LINELENGTH]; + char levelInfo[Amrvis::LINELENGTH]; char maxInfoV[Amrvis::LINELENGTH], minInfoV[Amrvis::LINELENGTH]; sprintf(levelInfo, "Level: %i", maxDrawnLevel); @@ -431,16 +432,16 @@ void Dataset::DatasetRender(const Box &alignedRegion, AmrPicture *apptr, XmStringFree(sNewLevel); sprintf(minInfoV, fstring, rMin); - // warning: '%s' directive writing up to 159 bytes into a region of size 156 [-Wformat-overflow=] - sprintf(minInfo, "Min:%s", minInfoV); - XmString sNewMin = XmStringCreateSimple(minInfo); + std::string minInfo("Min:"); + minInfo.append(minInfoV); + XmString sNewMin = XmStringCreateSimple(minInfo.c_str()); XtVaSetValues(wMinValue, XmNlabelString, sNewMin, NULL); XmStringFree(sNewMin); sprintf(maxInfoV, fstring, rMax); - // warning: '%s' directive writing up to 159 bytes into a region of size 156 [-Wformat-overflow=] - sprintf(maxInfo, "Max:%s", maxInfoV); - XmString sNewMax = XmStringCreateSimple(maxInfo); + std::string maxInfo("Max:"); + maxInfo.append(maxInfoV); + XmString sNewMax = XmStringCreateSimple(maxInfo.c_str()); XtVaSetValues(wMaxValue, XmNlabelString, sNewMax, NULL); XmStringFree(sNewMax); diff --git a/PltApp.cpp b/PltApp.cpp index f1cdc7d..a6dc800 100644 --- a/PltApp.cpp +++ b/PltApp.cpp @@ -42,6 +42,7 @@ #include #include +#include #include #include using std::cout; @@ -2815,7 +2816,6 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { return; } - char range[Amrvis::LINELENGTH]; char saveRangeString[Amrvis::LINELENGTH]; char format[Amrvis::LINELENGTH]; char fMin[Amrvis::LINELENGTH]; @@ -2832,9 +2832,9 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); - // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] - sprintf(range, "Min: %s Max: %s", fMin, fMax); - strcpy(saveRangeString, range); + std::string range("Min: "); + range.append(fMin).append(" Max: ").append(fMax); + strcpy(saveRangeString, range.c_str()); XtVaGetValues(wAmrVisTopLevel, XmNx, &xpos, @@ -2965,25 +2965,25 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { pltAppState->CurrentDerivedNumber(), rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); - // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] - sprintf(range, "Min: %s", fMin); - XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); + range = "Min: "; + range.append(fMin); + XtVaCreateManagedWidget(range.c_str(), xmLabelGadgetClass, wRangeRC, NULL); //XtVaSetValues(wid, XmNleftOffset, 20, NULL); - // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] - sprintf(range, "Max: %s", fMax); - XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); + range = "Max: "; + range.append(fMax); + XtVaCreateManagedWidget(range.c_str(), xmLabelGadgetClass, wRangeRC, NULL); pltAppState->GetMinMax(Amrvis::SUBREGIONMINMAX, currentFrame, pltAppState->CurrentDerivedNumber(), rtMin, rtMax); sprintf(fMin, format, rtMin); sprintf(fMax, format, rtMax); - // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] - sprintf(range, "Min: %s", fMin); - XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); - // warning: '%s' directive writing up to 159 bytes into a region of size 155 [-Wformat-overflow=] - sprintf(range, "Max: %s", fMax); - XtVaCreateManagedWidget(range, xmLabelGadgetClass, wRangeRC, NULL); + range = "Min: "; + range.append(fMin); + XtVaCreateManagedWidget(range.c_str(), xmLabelGadgetClass, wRangeRC, NULL); + range = "Max: "; + range.append(fMax); + XtVaCreateManagedWidget(range.c_str(), xmLabelGadgetClass, wRangeRC, NULL); pltAppState->GetMinMax(Amrvis::USERMINMAX, currentFrame, pltAppState->CurrentDerivedNumber(), From abcd1c54e497f190ff32454ed3d403b29f13ce69 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 10:56:09 -0700 Subject: [PATCH 4/7] XmStringCreateSimple: Pre-C++17 way of string.data() --- Dataset.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dataset.cpp b/Dataset.cpp index 69f7d73..b67122d 100644 --- a/Dataset.cpp +++ b/Dataset.cpp @@ -434,14 +434,14 @@ void Dataset::DatasetRender(const Box &alignedRegion, AmrPicture *apptr, sprintf(minInfoV, fstring, rMin); std::string minInfo("Min:"); minInfo.append(minInfoV); - XmString sNewMin = XmStringCreateSimple(minInfo.c_str()); + XmString sNewMin = XmStringCreateSimple(&minInfo[0]); XtVaSetValues(wMinValue, XmNlabelString, sNewMin, NULL); XmStringFree(sNewMin); sprintf(maxInfoV, fstring, rMax); std::string maxInfo("Max:"); maxInfo.append(maxInfoV); - XmString sNewMax = XmStringCreateSimple(maxInfo.c_str()); + XmString sNewMax = XmStringCreateSimple(&maxInfo[0]); XtVaSetValues(wMaxValue, XmNlabelString, sNewMax, NULL); XmStringFree(sNewMax); From 4763366aca204def0a17a08e445fb90cdba3d893 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 11:00:28 -0700 Subject: [PATCH 5/7] PltApp: More Updates --- PltApp.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/PltApp.cpp b/PltApp.cpp index a6dc800..51a8b26 100644 --- a/PltApp.cpp +++ b/PltApp.cpp @@ -2995,12 +2995,13 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { NULL); XtVaCreateManagedWidget("Min:", xmLabelGadgetClass, wid, NULL); //XtVaSetValues(wid, XmNmarginWidth, 0, NULL); - sprintf(range, format, rtMin); + char rangec[Amrvis::LINELENGTH]; + sprintf(rangec, format, rtMin); wUserMin = XtVaCreateManagedWidget("local range", xmTextFieldWidgetClass, wid, - XmNvalue, range, + XmNvalue, rangec, XmNeditable, true, - XmNcolumns, strlen(range)+2, + XmNcolumns, strlen(rangec)+2, NULL); AddStaticCallback(wUserMin, XmNactivateCallback, &PltApp::DoUserMin); @@ -3010,12 +3011,12 @@ void PltApp::DoSetRangeButton(Widget, XtPointer, XtPointer) { //XmNborderWidth, 0, NULL); XtVaCreateManagedWidget("Max:", xmLabelGadgetClass, wid, NULL); - sprintf(range, format, rtMax); + sprintf(rangec, format, rtMax); wUserMax = XtVaCreateManagedWidget("local range", xmTextFieldWidgetClass, wid, - XmNvalue, range, + XmNvalue, rangec, XmNeditable, true, - XmNcolumns, strlen(range)+2, + XmNcolumns, strlen(rangec)+2, NULL); AddStaticCallback(wUserMax, XmNactivateCallback, &PltApp::DoUserMax); From 4b61adb2ed69c675194861f2ea042d3e75a0086a Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 11:06:37 -0700 Subject: [PATCH 6/7] Fix Buffer Overflow in PltAppOutput.cpp --- PltAppOutput.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/PltAppOutput.cpp b/PltAppOutput.cpp index 03a6414..f4eb105 100644 --- a/PltAppOutput.cpp +++ b/PltAppOutput.cpp @@ -17,6 +17,8 @@ #include #include +#include + using std::cout; using std::cerr; using std::endl; @@ -61,14 +63,15 @@ void PltApp::DoOutput(Widget w, XtPointer data, XtPointer) { XtSetSensitive(XmSelectionBoxGetChild(wGetFileName, XmDIALOG_HELP_BUTTON), false); - char tempstr[Amrvis::BUFSIZE], tempfilename[Amrvis::BUFSIZE]; + char tempfilename[Amrvis::BUFSIZE]; if(animating2d) { strcpy(tempfilename, AVGlobals::StripSlashes(fileNames[currentFrame]).c_str()); } else { strcpy(tempfilename, AVGlobals::StripSlashes(fileNames[0]).c_str()); } - sprintf(tempstr, "%s_%s", pltAppState->CurrentDerived().c_str(), tempfilename); - XmTextSetString(XmSelectionBoxGetChild(wGetFileName, XmDIALOG_TEXT), tempstr); + std::string tempstr(pltAppState->CurrentDerived().c_str()); + tempstr.append("_").append(tempfilename); + XmTextSetString(XmSelectionBoxGetChild(wGetFileName, XmDIALOG_TEXT), &tempstr[0]); XtManageChild(wGetFileName); XtPopup(XtParent(wGetFileName), XtGrabNone); } // end DoOutput From 7c40d5a52e1c94ad49a8d09c07d50f32a8311331 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 17 Mar 2022 11:11:49 -0700 Subject: [PATCH 7/7] CI: Ignore AMReX Error for Now --- .github/workflows/ubuntu.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 5b189fe..cf15411 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -12,7 +12,9 @@ jobs: runs-on: ubuntu-20.04 if: github.event.pull_request.draft == false env: - CXXFLAGS: "-Werror" + # FIXME: -Wstringop-overflow + # https://github.com/AMReX-Codes/amrex/pull/2660 + CXXFLAGS: "-Werror -Wno-error=stringop-overflow" steps: - uses: actions/checkout@v3 - uses: actions/checkout@v3