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

Fix the compiling errors in gcc9 #1621

Merged
merged 4 commits into from
Feb 6, 2021

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Feb 2, 2021

What I did
Fix the following error.

    orch.cpp: In member function 'ref_resolve_status Orch::resolveFieldRefArray(type_map&, const string&, swss::KeyOpFieldsValuesTuple&, std::vector<long unsigned int>&, std::string&)':
    orch.cpp:609:41: error: 'strlen' argument missing terminating nul [-Werror=stringop-overflow=]
      609 |                     object_name_list += string(&list_item_delimiter);
          |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    In file included from orch.cpp:6:
    orch.h:25:12: note: referenced argument declared here
       25 | const char list_item_delimiter = ',';
          |            ^~~~~~~~~~~~~~~~~~~
    cc1plus: all warnings being treated as errors

Signed-off-by: Stephen Sun stephens@nvidia.com

Why I did it

How I verified it

Details if related

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

…iling error on gcc9

Add "-Wno-stringop-overflow" option to avoid the following error:

    orch.cpp: In member function 'ref_resolve_status Orch::resolveFieldRefArray(type_map&, const string&, swss::KeyOpFieldsValuesTuple&, std::vector<long unsigned int>&, std::string&)':
    orch.cpp:609:41: error: 'strlen' argument missing terminating nul [-Werror=stringop-overflow=]
      609 |                     object_name_list += string(&list_item_delimiter);
          |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    In file included from orch.cpp:6:
    orch.h:25:12: note: referenced argument declared here
       25 | const char list_item_delimiter = ',';
          |            ^~~~~~~~~~~~~~~~~~~
    cc1plus: all warnings being treated as errors

According to CauldronDevelopmentLLC/cbang#29, this is an error from boost and can be suppressed by using this option

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@qiluo-msft
Copy link
Contributor

It seems compiler report a true code bug.

const char list_item_delimiter = ',';
object_name_list += string(&list_item_delimiter)

Could you check why you feed a non zero-terminated char* to string ctor?

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comment

@stephenxs
Copy link
Collaborator Author

It seems compiler report a true code bug.

const char list_item_delimiter = ',';
object_name_list += string(&list_item_delimiter)

Could you check why you feed a non zero-terminated char* to string ctor?

const char list_item_delimiter = ',';
object_name_list += string(&list_item_delimiter, 1);

Need to add a count
Meanwhile, I don’t quite understand why it was not catched by gcc8

@qiluo-msft
Copy link
Contributor

This bug should be detected by either compiler or linter. Could you help explore a general solution to detect other unfound or future occurrences, during build process?

…the compiling error on gcc9"

This reverts commit 5d32b62.
Fix it by adding a "count" argument to the ctor of string

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@kcudnik
Copy link
Contributor

kcudnik commented Feb 3, 2021

change PR name to fix gcc9 warning

@stephenxs
Copy link
Collaborator Author

change PR name to fix gcc9 warning

Done.

@kcudnik
Copy link
Contributor

kcudnik commented Feb 3, 2021

This bug should be detected by either compiler or linter. Could you help explore a general solution to detect other unfound or future occurrences, during build process?

i actually checked g++ without and with O2 optimization, and if you use & on char constant, it puts data as a single character string which is padded by zeros, and with O2 even aligned with 0x10, but it could be just good practice by gcc, it may not be guaranteed, so warning is helpful in that case

@stephenxs stephenxs changed the title Add diagnose option "-Wno-stringop-overflow" option to avoid the compiling errors in order to avoid error in gcc9 Fix the compiling errors in gcc9 Feb 3, 2021
orchagent/orch.cpp Outdated Show resolved Hide resolved
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Collaborator Author

This bug should be detected by either compiler or linter. Could you help explore a general solution to detect other unfound or future occurrences, during build process?

i actually checked g++ without and with O2 optimization, and if you use & on char constant, it puts data as a single character string which is padded by zeros, and with O2 even aligned with 0x10, but it could be just good practice by gcc, it may not be guaranteed, so warning is helpful in that case

I'm not sure whether it's related to optimization. I think the default alignment of the types whose size is less than a word length will be the word length of the machine, unless preprecess instructions like #progma pack(1) is used. This means there will be padding bytes after a char variable. But there isn't guarantee that the padding bytes will be zero.

@kcudnik kcudnik merged commit 1438a70 into sonic-net:master Feb 6, 2021
@stephenxs stephenxs deleted the no-stringop-overflow branch February 6, 2021 09:43
daall pushed a commit that referenced this pull request Feb 16, 2021
DavidZagury pushed a commit to DavidZagury/sonic-swss that referenced this pull request Mar 4, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants