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

Set up CodeQL scans and fixed several findings #1601

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: "CodeQL"

on:
push:
branches: [ master ]
pull_request:
branches: [ master ]
schedule:
- cron: '43 10 * * 2'

jobs:
analyze:
name: Analyze
runs-on: ubuntu-latest
container: ultimaker/cura-build-environment
permissions:
actions: read
contents: read
security-events: write

strategy:
fail-fast: false
matrix:
language: [ 'cpp' ]

steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Initialize CodeQL
uses: github/codeql-action/init@v1
with:
languages: ${{ matrix.language }}

- name: Build
run: docker/build.sh
env:
TESTS: OFF

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v1
3 changes: 2 additions & 1 deletion cmake/FindStb.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ else()
# Stb's commits in early February seems to cause the engine to fail compilation on Mac.
ExternalProject_Add(stb
GIT_REPOSITORY "https://github.com/nothings/stb.git"
GIT_TAG d5d052c806eee2ca1f858cb58b2f062d9fa25b90
GIT_TAG af1a5bc352164740c1cc1354942b1c6b72eacb8a
PATCH_COMMAND git apply ${CMAKE_SOURCE_DIR}/cmake/stb_image.h.patch # This patch fixes several integer overflows in stb_image.h. This can be removed once https://github.com/nothings/stb/pull/1306 is merged.
UPDATE_DISCONNECTED TRUE
CONFIGURE_COMMAND "" #We don't want to actually go and build/test/generate it. Just need to download the headers.
BUILD_COMMAND ""
Expand Down
72 changes: 72 additions & 0 deletions cmake/stb_image.h.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
diff --git a/stb_image.h b/stb_image.h
index d60371b..16e2158 100644
--- a/stb_image.h
+++ b/stb_image.h
@@ -1797,7 +1797,7 @@ static stbi__uint16 *stbi__convert_format16(stbi__uint16 *data, int img_n, int r
if (req_comp == img_n) return data;
STBI_ASSERT(req_comp >= 1 && req_comp <= 4);

- good = (stbi__uint16 *) stbi__malloc(req_comp * x * y * 2);
+ good = (stbi__uint16 *) stbi__malloc((size_t) req_comp * x * y * 2);
if (good == NULL) {
STBI_FREE(data);
return (stbi__uint16 *) stbi__errpuc("outofmem", "Out of memory");
@@ -1872,7 +1872,7 @@ static stbi_uc *stbi__hdr_to_ldr(float *data, int x, int y, int comp)
if (comp & 1) n = comp; else n = comp-1;
for (i=0; i < x*y; ++i) {
for (k=0; k < n; ++k) {
- float z = (float) pow(data[i*comp+k]*stbi__h2l_scale_i, stbi__h2l_gamma_i) * 255 + 0.5f;
+ float z = (float) pow((double) data[i*comp+k]*stbi__h2l_scale_i, stbi__h2l_gamma_i) * 255 + 0.5f;
if (z < 0) z = 0;
if (z > 255) z = 255;
output[i*comp + k] = (stbi_uc) stbi__float2int(z);
@@ -6123,7 +6123,7 @@ static void *stbi__psd_load(stbi__context *s, int *x, int *y, int *comp, int req
out = (stbi_uc *) stbi__malloc_mad3(8, w, h, 0);
ri->bits_per_channel = 16;
} else
- out = (stbi_uc *) stbi__malloc(4 * w*h);
+ out = (stbi_uc *) stbi__malloc((size_t) 4 * w*h);

if (!out) return stbi__errpuc("outofmem", "Out of memory");
pixelCount = w*h;
@@ -6446,7 +6446,7 @@ static void *stbi__pic_load(stbi__context *s,int *px,int *py,int *comp,int req_c
// intermediate buffer is RGBA
result = (stbi_uc *) stbi__malloc_mad3(x, y, 4, 0);
if (!result) return stbi__errpuc("outofmem", "Out of memory");
- memset(result, 0xff, x*y*4);
+ memset(result, 0xff, (size_t) x*y*4);

if (!stbi__pic_load_core(s,x,y,comp, result)) {
STBI_FREE(result);
@@ -6755,11 +6755,11 @@ static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, i
}

// background is what out is after the undoing of the previou frame;
- memcpy( g->background, g->out, 4 * g->w * g->h );
+ memcpy( g->background, g->out, (size_t) 4 * g->w * g->h );
}

// clear my history;
- memset( g->history, 0x00, g->w * g->h ); // pixels that were affected previous frame
+ memset( g->history, 0x00, (size_t) g->w * g->h ); // pixels that were affected previous frame

for (;;) {
int tag = stbi__get8(s);
@@ -6913,7 +6913,7 @@ static void *stbi__load_gif_main(stbi__context *s, int **delays, int *x, int *y,
stride = g.w * g.h * 4;

if (out) {
- void *tmp = (stbi_uc*) STBI_REALLOC_SIZED( out, out_size, layers * stride );
+ void *tmp = (stbi_uc*) STBI_REALLOC_SIZED( out, out_size, (size_t) layers * stride );
if (!tmp)
return stbi__load_gif_main_outofmem(&g, out, delays);
else {
@@ -6929,7 +6929,7 @@ static void *stbi__load_gif_main(stbi__context *s, int **delays, int *x, int *y,
delays_size = layers * sizeof(int);
}
} else {
- out = (stbi_uc*)stbi__malloc( layers * stride );
+ out = (stbi_uc*)stbi__malloc((size_t) layers * stride );
if (!out)
return stbi__load_gif_main_outofmem(&g, out, delays);
out_size = layers * stride;
11 changes: 9 additions & 2 deletions docker/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,21 @@ export PATH="${CURA_BUILD_ENV_PATH}/bin:${PATH}"
export PKG_CONFIG_PATH="${CURA_BUILD_ENV_PATH}/lib/pkgconfig:${PKG_CONFIG_PATH}"
export LD_LIBRARY_PATH="${CURA_BUILD_ENV_PATH}/lib:${LD_LIBRARY_PATH}"

if [ "${TESTS}" = "" ]; then
TESTS="ON"
fi

cd "${PROJECT_DIR}"

mkdir build
cd build
cmake \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_PREFIX_PATH="${CURA_BUILD_ENV_PATH}" \
-DBUILD_TESTS=ON \
-DBUILD_TESTS=${TESTS} \
..
make
ctest --output-on-failure -T Test

if [ "${TESTS}" = "ON" ]; then
ctest --output-on-failure -T Test
fi
8 changes: 4 additions & 4 deletions src/BeadingStrategy/BeadingStrategyFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,21 @@ BeadingStrategyPtr BeadingStrategyFactory::makeStrategy

if(print_thin_walls)
{
logDebug("Applying the Widening Beading meta-strategy with minimum input width %d and minimum output width %d.\n", min_feature_size, min_bead_width);
logDebug("Applying the Widening Beading meta-strategy with minimum input width %lld and minimum output width %lld.\n", min_feature_size, min_bead_width);
ret = make_unique<WideningBeadingStrategy>(move(ret), min_feature_size, min_bead_width);
}
if (max_bead_count > 0)
{
logDebug("Applying the Redistribute meta-strategy with outer-wall width = %d, inner-wall width = %d\n", preferred_bead_width_outer, preferred_bead_width_inner);
logDebug("Applying the Redistribute meta-strategy with outer-wall width = %lld, inner-wall width = %lld\n", preferred_bead_width_outer, preferred_bead_width_inner);
ret = make_unique<RedistributeBeadingStrategy>(preferred_bead_width_outer, preferred_bead_width_inner, minimum_variable_line_width, move(ret));
//Apply the LimitedBeadingStrategy last, since that adds a 0-width marker wall which other beading strategies shouldn't touch.
logDebug("Applying the Limited Beading meta-strategy with maximum bead count = %d.\n", max_bead_count);
logDebug("Applying the Limited Beading meta-strategy with maximum bead count = %lld.\n", max_bead_count);
ret = make_unique<LimitedBeadingStrategy>(max_bead_count, move(ret));
}

if (outer_wall_offset > 0)
{
logDebug("Applying the OuterWallOffset meta-strategy with offset = %d.\n", outer_wall_offset);
logDebug("Applying the OuterWallOffset meta-strategy with offset = %lld.\n", outer_wall_offset);
ret = make_unique<OuterWallInsetBeadingStrategy>(outer_wall_offset, move(ret));
}
return ret;
Expand Down
2 changes: 1 addition & 1 deletion src/FffGcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ void FffGcodeWriter::processRaft(const SliceDataStorage& storage)

LayerPlan& FffGcodeWriter::processLayer(const SliceDataStorage& storage, LayerIndex layer_nr, const size_t total_layers) const
{
logDebug("GcodeWriter processing layer %i of %i\n", layer_nr, total_layers);
logDebug("GcodeWriter processing layer %i of %lu\n", (int) layer_nr, total_layers);

const Settings& mesh_group_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings;
coord_t layer_thickness = mesh_group_settings.get<coord_t>("layer_height");
Expand Down
12 changes: 6 additions & 6 deletions src/FffPolygonGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,15 @@ bool FffPolygonGenerator::sliceModel(MeshGroup* meshgroup, TimeKeeper& timeKeepe
coord_t initial_layer_thickness = mesh_group_settings.get<coord_t>("layer_height_0");
if (initial_layer_thickness <= 0)
{
logError("Initial layer height %i is disallowed.\n", initial_layer_thickness);
logError("Initial layer height %lld is disallowed.\n", initial_layer_thickness);
return false;
}

// Layer height of 0 is not allowed. Negative layer height is nonsense.
const coord_t layer_thickness = mesh_group_settings.get<coord_t>("layer_height");
if(layer_thickness <= 0)
{
logError("Layer height %i is disallowed.\n", layer_thickness);
logError("Layer height %lld is disallowed.\n", layer_thickness);
return false;
}

Expand Down Expand Up @@ -380,7 +380,7 @@ void FffPolygonGenerator::slices2polygons(SliceDataStorage& storage, TimeKeeper&
removeEmptyFirstLayers(storage, storage.print_layer_count); // changes storage.print_layer_count!
}

log("Layer count: %i\n", storage.print_layer_count);
log("Layer count: %lu\n", storage.print_layer_count);

//layerparts2HTML(storage, "output/output.html");

Expand Down Expand Up @@ -466,7 +466,7 @@ void FffPolygonGenerator::processBasicWallsSkinInfill(SliceDataStorage& storage,
// Use a signed type for the loop counter so MSVC compiles (because it uses OpenMP 2.0, an old version).
for (int layer_number = 0; layer_number < static_cast<int>(mesh.layers.size()); layer_number++)
{
logDebug("Processing insets for layer %i of %i\n", layer_number, mesh_layer_count);
logDebug("Processing insets for layer %i of %lu\n", layer_number, mesh_layer_count);
processWalls(mesh, layer_number);
#ifdef _OPENMP
if (omp_get_thread_num() == 0)
Expand Down Expand Up @@ -525,7 +525,7 @@ void FffPolygonGenerator::processBasicWallsSkinInfill(SliceDataStorage& storage,
// Use a signed type for the loop counter so MSVC compiles (because it uses OpenMP 2.0, an old version).
for (int layer_number = 0; layer_number < static_cast<int>(mesh.layers.size()); layer_number++)
{
logDebug("Processing skins and infill layer %i of %i\n", layer_number, mesh_layer_count);
logDebug("Processing skins and infill layer %i of %lu\n", layer_number, mesh_layer_count);
if (!mesh_group_settings.get<bool>("magic_spiralize") || layer_number < static_cast<int>(mesh_max_initial_bottom_layer_count)) //Only generate up/downskin and infill for the first X layers when spiralize is choosen.
{
processSkinsAndInfill(mesh, layer_number, process_infill);
Expand Down Expand Up @@ -778,7 +778,7 @@ void FffPolygonGenerator::removeEmptyFirstLayers(SliceDataStorage& storage, size

if (n_empty_first_layers > 0)
{
log("Removing %d layers because they are empty\n", n_empty_first_layers);
log("Removing %lu layers because they are empty\n", n_empty_first_layers);
const coord_t layer_height = Application::getInstance().current_slice->scene.current_mesh_group->settings.get<coord_t>("layer_height");
for (SliceMeshStorage& mesh : storage.meshes)
{
Expand Down
10 changes: 5 additions & 5 deletions src/LayerPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,9 @@ void LayerPlan::addPolygonsByOptimizer(const Polygons& polygons, const GCodePath
}
}

static constexpr float max_non_bridge_line_volume = MM2INT(100); // limit to accumulated "volume" of non-bridge lines which is proportional to distance x extrusion rate
static constexpr double max_non_bridge_line_volume = MM2INT(100); // limit to accumulated "volume" of non-bridge lines which is proportional to distance x extrusion rate

void LayerPlan::addWallLine(const Point& p0, const Point& p1, const Settings& settings, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, float flow, float& non_bridge_line_volume, Ratio speed_factor, double distance_to_bridge_start)
void LayerPlan::addWallLine(const Point& p0, const Point& p1, const Settings& settings, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, float flow, double& non_bridge_line_volume, Ratio speed_factor, double distance_to_bridge_start)
{
const coord_t min_line_len = 5; // we ignore lines less than 5um long
const double acceleration_segment_len = MM2INT(1); // accelerate using segments of this length
Expand Down Expand Up @@ -672,7 +672,7 @@ void LayerPlan::addWallLine(const Point& p0, const Point& p1, const Settings& se
addExtrusionMove(segment_end, non_bridge_config, SpaceFillType::Polygons, segment_flow, spiralize,
(overhang_mask.empty() || (!overhang_mask.inside(p0, true) && !overhang_mask.inside(p1, true))) ? speed_factor : overhang_speed_factor);
}
non_bridge_line_volume += vSize(cur_point - segment_end) * segment_flow * speed_factor * non_bridge_config.getSpeed();
non_bridge_line_volume += (double) vSize(cur_point - segment_end) * segment_flow * speed_factor * non_bridge_config.getSpeed();
cur_point = segment_end;
speed_factor = 1 - (1 - speed_factor) * acceleration_factor;
if (speed_factor >= 0.9)
Expand Down Expand Up @@ -814,7 +814,7 @@ void LayerPlan::addWall(const ExtrusionLine& wall, int start_idx, const Settings
start_idx = locateFirstSupportedVertex(wall, start_idx);
}

float non_bridge_line_volume = max_non_bridge_line_volume; // assume extruder is fully pressurised before first non-bridge line is output
double non_bridge_line_volume = max_non_bridge_line_volume; // assume extruder is fully pressurised before first non-bridge line is output
double speed_factor = 1.0; // start first line at normal speed
coord_t distance_to_bridge_start = 0; // will be updated before each line is processed

Expand Down Expand Up @@ -1191,7 +1191,7 @@ void LayerPlan::addLinesInGivenOrder(
ConstPolygonRef next_polygon = *next_path.vertices;
const size_t next_start = next_path.start_vertex;
const Point& next_p0 = next_polygon[next_start];
if (vSize2(next_p0 - p1) <= line_width * line_width * 4)
if (vSize2(next_p0 - p1) <= (long) line_width * line_width * 4)
{
wipe = false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/LayerPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ class LayerPlan : public NoCopy
* \param distance_to_bridge_start The distance along the wall from p0 to the first bridge segment
*/

void addWallLine(const Point& p0, const Point& p1, const Settings& settings, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, float flow, float& non_bridge_line_volume, Ratio speed_factor, double distance_to_bridge_start);
void addWallLine(const Point& p0, const Point& p1, const Settings& settings, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, float flow, double& non_bridge_line_volume, Ratio speed_factor, double distance_to_bridge_start);

/*!
* Add a wall to the g-code starting at vertex \p start_idx
Expand Down
2 changes: 1 addition & 1 deletion src/LayerPlanBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer

if (!new_layer_destination_state)
{
logWarning("Layer %d is empty (or it has empty extruder plans). Temperature control and cross layer travel moves might suffer!\n", newest_layer->layer_nr);
logWarning("Layer %d is empty (or it has empty extruder plans). Temperature control and cross layer travel moves might suffer!\n", (int) newest_layer->layer_nr);
return;
}

Expand Down
8 changes: 4 additions & 4 deletions src/SkeletalTrapezoidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1635,9 +1635,9 @@ SkeletalTrapezoidation::Beading SkeletalTrapezoidation::interpolate(const Beadin
}
else
{
ret.bead_widths[inset_idx] = ratio_left_to_whole * left.bead_widths[inset_idx] + ratio_right_to_whole * right.bead_widths[inset_idx];
ret.bead_widths[inset_idx] = ratio_left_to_whole * left.bead_widths[inset_idx] + (double) ratio_right_to_whole * right.bead_widths[inset_idx];
}
ret.toolpath_locations[inset_idx] = ratio_left_to_whole * left.toolpath_locations[inset_idx] + ratio_right_to_whole * right.toolpath_locations[inset_idx];
ret.toolpath_locations[inset_idx] = ratio_left_to_whole * left.toolpath_locations[inset_idx] + (double) ratio_right_to_whole * right.toolpath_locations[inset_idx];
}
return ret;
}
Expand Down Expand Up @@ -1925,7 +1925,7 @@ void SkeletalTrapezoidation::connectJunctions(ptr_vector_t<LineJunctions>& edge_
assert(std::abs(int(from_junctions.size()) - int(to_junctions.size())) <= 1); // at transitions one end has more beads
if(std::abs(int(from_junctions.size()) - int(to_junctions.size())) > 1)
{
logWarning("Can't create a transition when connecting two perimeters where the number of beads differs too much! %i vs. %i", from_junctions.size(), to_junctions.size());
logWarning("Can't create a transition when connecting two perimeters where the number of beads differs too much! %lu vs. %lu", from_junctions.size(), to_junctions.size());
}

size_t segment_count = std::min(from_junctions.size(), to_junctions.size());
Expand All @@ -1936,7 +1936,7 @@ void SkeletalTrapezoidation::connectJunctions(ptr_vector_t<LineJunctions>& edge_
assert(from.perimeter_index == to.perimeter_index);
if(from.perimeter_index != to.perimeter_index)
{
logWarning("Connecting two perimeters with different indices! Perimeter %i and %i", from.perimeter_index, to.perimeter_index);
logWarning("Connecting two perimeters with different indices! Perimeter %lu and %lu", from.perimeter_index, to.perimeter_index);
}
const bool from_is_odd =
quad_start->to->data.bead_count > 0 && quad_start->to->data.bead_count % 2 == 1 // quad contains single bead segment
Expand Down
4 changes: 2 additions & 2 deletions src/Weaver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void Weaver::weave(MeshGroup* meshgroup)
const size_t layer_count = (maxz - initial_layer_thickness) / connection_height + 1;
std::vector<AdaptiveLayer> layer_thicknesses;

log("Layer count: %i\n", layer_count);
log("Layer count: %lu\n", layer_count);

std::vector<cura::Slicer*> slicerList;

Expand All @@ -54,7 +54,7 @@ void Weaver::weave(MeshGroup* meshgroup)
}
if (starting_layer_idx > 0)
{
logWarning("First %i layers are empty!\n", starting_layer_idx);
logWarning("First %i layers are empty!\n", (int) starting_layer_idx);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/communication/ArcusCommunication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,11 @@ void ArcusCommunication::sendOptimizedLayerData()
{
return;
}
log("Sending %d layers.", data.current_layer_count);
log("Sending %lu layers.", data.current_layer_count);

for (std::pair<const int, std::shared_ptr<proto::LayerOptimized>> entry : data.slice_data) //Note: This is in no particular order!
{
logDebug("Sending layer data for layer %i of %i.\n", entry.first, data.slice_data.size());
logDebug("Sending layer data for layer %i of %lu.\n", entry.first, data.slice_data.size());
private_data->socket->sendMessage(entry.second); //Send the actual layers.
}
data.sliced_objects = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/gcodeExport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ void GCodeExport::finalize(const char* endCode)
writeCode(endCode);
int64_t print_time = getSumTotalPrintTimes();
int mat_0 = getTotalFilamentUsed(0);
log("Print time (s): %d\n", print_time);
log("Print time (s): %l\n", print_time);
log("Print time (hr|min|s): %dh %dm %ds\n", int(print_time / 60 / 60), int((print_time / 60) % 60), int(print_time % 60));
log("Filament (mm^3): %d\n", mat_0);
for(int n=1; n<MAX_EXTRUDERS; n++)
Expand Down
Loading