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.

Sign up for GitHub

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

platform.h: fixed clang-analyzer-core.BitwiseShift warnings and adjusted types of *_bit members #7271

Merged
merged 2 commits into from
Feb 5, 2025
Merged

platform.h: fixed clang-analyzer-core.BitwiseShift warnings and adjusted types of *_bit members #7271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3e77a1
platform.h: fixed `clang-analyzer-core.BitwiseShift` warnings and adj…
firewave Jan 29, 2025
7d83dca
cppcheck.cpp: fixed writing to `Platform::*_bit` to dump file
firewave Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/checkcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1944,7 +1944,7 @@ void CheckCondition::checkCompareValueOutOfTypeRange()
continue;
if (valueTok->valueType() && valueTok->valueType()->isTypeEqual(typeTok->valueType()))
continue;
int bits = 0;
std::uint8_t bits = 0;
switch (typeTok->valueType()->type) {
case ValueType::Type::BOOL:
bits = 1;
Expand Down
2 changes: 1 addition & 1 deletion lib/checktype.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void CheckType::checkTooBigBitwiseShift()
// C11 Standard, section 6.5.7 Bitwise shift operators, states:
// The integer promotions are performed on each of the operands.
// The type of the result is that of the promoted left operand.
int lhsbits;
std::uint8_t lhsbits;
if ((lhstype->type == ValueType::Type::CHAR) ||
(lhstype->type == ValueType::Type::SHORT) ||
(lhstype->type == ValueType::Type::WCHAR_T) ||
Expand Down
10 changes: 5 additions & 5 deletions lib/cppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,11 @@ static void createDumpFile(const Settings& settings,
fdump << "<dumps" << language << ">\n";
fdump << " <platform"
<< " name=\"" << settings.platform.toString() << '\"'
<< " char_bit=\"" << settings.platform.char_bit << '\"'
<< " short_bit=\"" << settings.platform.short_bit << '\"'
<< " int_bit=\"" << settings.platform.int_bit << '\"'
<< " long_bit=\"" << settings.platform.long_bit << '\"'
<< " long_long_bit=\"" << settings.platform.long_long_bit << '\"'
<< " char_bit=\"" << static_cast<unsigned>(settings.platform.char_bit) << '\"'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

uint8_t is treated as a character with stream insertion operators. This caused invalid dump files to be written.

First, we should not write these files raw but using TinyXML2.

Second, I wonder if it would make sense to have a check which detects the usage of uint8_t in stream insertion operators. If you want it to a be an actual character you should probably use {unsigned|signed} char or char8_t.

Copy link
Owner

Choose a reason for hiding this comment

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

Second, I wonder if it would make sense to have a check which detects the usage of uint8_t in stream insertion operators. If you want it to a be an actual character you should probably use {unsigned|signed} char or char8_t.

Spontanously that checker sounds reasonable to me.

<< " short_bit=\"" << static_cast<unsigned>(settings.platform.short_bit) << '\"'
<< " int_bit=\"" << static_cast<unsigned>(settings.platform.int_bit) << '\"'
<< " long_bit=\"" << static_cast<unsigned>(settings.platform.long_bit) << '\"'
<< " long_long_bit=\"" << static_cast<unsigned>(settings.platform.long_long_bit) << '\"'
<< " pointer_bit=\"" << (settings.platform.sizeof_pointer * settings.platform.char_bit) << '\"'
<< " wchar_t_bit=\"" << (settings.platform.sizeof_wchar_t * settings.platform.char_bit) << '\"'
<< " size_t_bit=\"" << (settings.platform.sizeof_size_t * settings.platform.char_bit) << '\"'
Expand Down
20 changes: 12 additions & 8 deletions lib/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "mathlib.h"
#include "standards.h"

#include <cassert>
#include <climits>
#include <cstddef>
#include <cstdint>
Expand All @@ -44,19 +45,22 @@ namespace tinyxml2 {
*/
class CPPCHECKLIB Platform {
private:
static long long min_value(int bit) {
static long long min_value(std::uint8_t bit) {
assert(bit > 0);
if (bit >= 64)
return LLONG_MIN;
return -(1LL << (bit-1));
}

static long long max_value(int bit) {
static long long max_value(std::uint8_t bit) {
assert(bit > 0);
if (bit >= 64)
return (~0ULL) >> 1;
return (1LL << (bit-1)) - 1LL;
}

static unsigned long long max_value_unsigned(int bit) {
static unsigned long long max_value_unsigned(std::uint8_t bit) {
assert(bit > 0);
if (bit >= 64)
return ~0ULL;
return (1ULL << bit) - 1ULL;
Expand Down Expand Up @@ -90,11 +94,11 @@ class CPPCHECKLIB Platform {
return value <= longLongMax;
}

nonneg int char_bit; /// bits in char
nonneg int short_bit; /// bits in short
nonneg int int_bit; /// bits in int
nonneg int long_bit; /// bits in long
nonneg int long_long_bit; /// bits in long long
std::uint8_t char_bit; /// bits in char
std::uint8_t short_bit; /// bits in short
std::uint8_t int_bit; /// bits in int
std::uint8_t long_bit; /// bits in long
std::uint8_t long_long_bit; /// bits in long long

/** size of standard types */
std::size_t sizeof_bool;
Expand Down
2 changes: 1 addition & 1 deletion lib/symboldatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1848,7 +1848,7 @@ void SymbolDatabase::setArrayDimensionsUsingValueFlow()
}

if (dimension.tok->valueType() && dimension.tok->valueType()->pointer == 0) {
int bits = 0;
std::uint8_t bits = 0;
switch (dimension.tok->valueType()->type) {
case ValueType::Type::CHAR:
bits = mSettings.platform.char_bit;
Expand Down
4 changes: 2 additions & 2 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,13 +959,13 @@ static void valueFlowRightShift(TokenList& tokenList, const Settings& settings)
continue;
if (lhsmax < 0)
continue;
int lhsbits;
std::uint8_t lhsbits;
if ((tok->astOperand1()->valueType()->type == ValueType::Type::CHAR) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::SHORT) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::WCHAR_T) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::BOOL) ||
(tok->astOperand1()->valueType()->type == ValueType::Type::INT))
lhsbits = settings.platform.int_bit;
lhsbits = settings.platform.int_bit; // TODO: needs to use the proper *_bit fo each type
else if (tok->astOperand1()->valueType()->type == ValueType::Type::LONG)
lhsbits = settings.platform.long_bit;
else if (tok->astOperand1()->valueType()->type == ValueType::Type::LONGLONG)
Expand Down
2 changes: 1 addition & 1 deletion lib/vf_common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace ValueFlow
if (!vt || !vt->isIntegral() || vt->pointer)
return false;

int bits;
std::uint8_t bits;
switch (vt->type) {
case ValueType::Type::BOOL:
bits = 1;
Expand Down
4 changes: 2 additions & 2 deletions lib/vf_settokenvalue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ namespace ValueFlow
continue;
Value v(val);
v.intvalue = ~v.intvalue;
int bits = 0;
std::uint8_t bits = 0;
if (tok->valueType() &&
tok->valueType()->sign == ValueType::Sign::UNSIGNED &&
tok->valueType()->pointer == 0) {
Expand All @@ -618,7 +618,7 @@ namespace ValueFlow
bits = settings.platform.long_bit;
}
if (bits > 0 && bits < MathLib::bigint_bits)
v.intvalue &= (((MathLib::biguint)1)<<bits) - 1;
v.intvalue &= (1ULL<<bits) - 1;
Copy link
Owner

@danmar danmar Feb 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

after your changes this would not work well the day we switch to 128-bit bigints.. however I guess there are plenty of other code that will not work perfectly that day.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This actually causes a compilation failure when using the 128-bit int. That's how I came across this.

setTokenValue(parent, std::move(v), settings);
}
}
Expand Down
Loading