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

Enable more compiler warnings #480

Merged
merged 7 commits into from
Aug 13, 2024
Merged
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
62 changes: 61 additions & 1 deletion src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,67 @@ ifeq ($(PLATFORM),Darwin)
endif

# The base compiler flags. More can be added on command line.
CFLAGS += -I. -I../inc -O2 -Wall -Wextra
CFLAGS += -I. -I../inc -O2
# Enable a bunch of optional warnings.
CFLAGS += \
-pedantic \
-Wall \
-Wextra \
-Waggregate-return \
-Walloca \
-Warray-bounds \
-Wbad-function-cast \
-Wc++-compat \
-Wc++11-compat \
-Wcast-align \
-Wcast-qual \
-Wconversion \
-Wdisabled-optimization \
-Wdouble-promotion \
-Wenum-compare \
-Wfloat-equal \
-Wformat-nonliteral \
-Wformat-security \
-Wformat-y2k \
-Wformat=2 \
-Wframe-larger-than=1048576 \
-Wimplicit \
-Wimplicit-fallthrough \
-Winit-self \
-Winline \
-Winvalid-pch \
-Wlarger-than=512 \
-Wlong-long \
-Wmissing-declarations \
-Wmissing-field-initializers \
-Wmissing-format-attribute \
-Wmissing-include-dirs \
-Wmissing-noreturn \
-Wmissing-prototypes \
-Wnested-externs \
-Wnull-dereference \
-Wold-style-definition \
-Woverlength-strings \
-Wpacked \
-Wpadded \
-Wpointer-arith \
-Wredundant-decls \
-Wredundant-move \
-Wshadow \
-Wsign-compare \
-Wsign-conversion \
-Wstack-protector \
-Wstrict-aliasing=2 \
-Wstrict-overflow=5 \
-Wstrict-prototypes \
-Wswitch-default \
-Wswitch-enum \
-Wtype-limits \
-Wundef \
-Wuninitialized \
-Wunreachable-code \
-Wvariadic-macros \
-Wwrite-strings

# Cross-platform compilation settings.
ifeq ($(PLATFORM),Windows)
Expand Down
4 changes: 2 additions & 2 deletions src/common/lincomb.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void g1_lincomb_naive(g1_t *out, const g1_t *p, const fr_t *coeffs, uint64_t len
*/
C_KZG_RET g1_lincomb_fast(g1_t *out, const g1_t *p, const fr_t *coeffs, size_t len) {
C_KZG_RET ret;
void *scratch = NULL;
limb_t *scratch = NULL;
blst_p1 *p_filtered = NULL;
blst_p1_affine *p_affine = NULL;
blst_scalar *scalars = NULL;
Expand All @@ -88,7 +88,7 @@ C_KZG_RET g1_lincomb_fast(g1_t *out, const g1_t *p, const fr_t *coeffs, size_t l

/* Allocate space for Pippenger scratch */
size_t scratch_size = blst_p1s_mult_pippenger_scratch_sizeof(len);
ret = c_kzg_malloc(&scratch, scratch_size);
ret = c_kzg_malloc((void **)&scratch, scratch_size);
if (ret != C_KZG_OK) goto out;

/* Transform the field elements to 256-bit scalars */
Expand Down
38 changes: 19 additions & 19 deletions src/common/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "common/utils.h"
#include "common/alloc.h"

#include <stddef.h> /* For size_t */
#include <stdlib.h> /* For NULL */
#include <string.h> /* For memcpy */

Expand All @@ -43,26 +44,26 @@ bool is_power_of_two(uint64_t n) {
* @return The log base two of n.
*
* @remark In other words, the bit index of the one bit.
* @remark Works only for n a power of two, and only for n up to 2^31.
* @remark Works only for n a power of two.
* @remark Not the fastest implementation, but it doesn't need to be fast.
*/
int log2_pow2(uint32_t n) {
int position = 0;
uint64_t log2_pow2(uint64_t n) {
uint64_t position = 0;
while (n >>= 1)
position++;
return position;
}

/**
* Reverse the bit order in a 32-bit integer.
* Reverse the bit order in a 64-bit integer.
*
* @param[in] n The integer to be reversed
*
* @return An integer with the bits of `n` reversed.
*/
uint32_t reverse_bits(uint32_t n) {
uint32_t result = 0;
for (int i = 0; i < 32; ++i) {
uint64_t reverse_bits(uint64_t n) {
uint64_t result = 0;
for (size_t i = 0; i < 64; ++i) {
result <<= 1;
result |= (n & 1);
n >>= 1;
Expand All @@ -71,7 +72,7 @@ uint32_t reverse_bits(uint32_t n) {
}

/**
* Reverse the low-order bits in a 32-bit integer.
* Reverse the low-order bits in a 64-bit integer.
*
* @param[in] n To reverse `b` bits, set `n = 2 ^ b`
* @param[in] value The bits to be reversed
Expand All @@ -80,8 +81,8 @@ uint32_t reverse_bits(uint32_t n) {
*
* @remark n must be a power of two.
*/
uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
size_t unused_bit_len = 32 - log2_pow2(n);
uint64_t reverse_bits_limited(uint64_t n, uint64_t value) {
size_t unused_bit_len = 64 - log2_pow2(n);
return reverse_bits(value) >> unused_bit_len;
}

Expand All @@ -90,8 +91,7 @@ uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
*
* @param[in,out] values The array, which is re-ordered in-place
* @param[in] size The size in bytes of an element of the array
* @param[in] n The length of the array, must be a power of two
* strictly greater than 1 and less than 2^32.
* @param[in] n The length of the array, must be a power of two strictly greater than 1
*
* @remark Operates in-place on the array.
* @remark Can handle arrays of any type: provide the element size in `size`.
Expand All @@ -102,10 +102,10 @@ uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
C_KZG_RET ret;
byte *tmp = NULL;
byte *v = values;
byte *v = (byte *)values;

/* Some sanity checks */
if (n < 2 || n >= UINT32_MAX || !is_power_of_two(n)) {
if (n < 2 || !is_power_of_two(n)) {
ret = C_KZG_BADARGS;
goto out;
}
Expand All @@ -115,9 +115,9 @@ C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
if (ret != C_KZG_OK) goto out;

/* Reorder elements */
int unused_bit_len = 32 - log2_pow2(n);
for (uint32_t i = 0; i < n; i++) {
uint32_t r = reverse_bits(i) >> unused_bit_len;
uint64_t unused_bit_len = 64 - log2_pow2(n);
for (uint64_t i = 0; i < n; i++) {
uint64_t r = reverse_bits(i) >> unused_bit_len;
if (r > i) {
/* Swap the two elements */
memcpy(tmp, v + (i * size), size);
Expand All @@ -140,9 +140,9 @@ C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
*
* @remark `out` is left untouched if `n == 0`.
*/
void compute_powers(fr_t *out, const fr_t *x, uint64_t n) {
void compute_powers(fr_t *out, const fr_t *x, size_t n) {
fr_t current_power = FR_ONE;
for (uint64_t i = 0; i < n; i++) {
for (size_t i = 0; i < n; i++) {
out[i] = current_power;
blst_fr_mul(&current_power, &current_power, x);
}
Expand Down
8 changes: 4 additions & 4 deletions src/common/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ extern "C" {
#endif

bool is_power_of_two(uint64_t n);
int log2_pow2(uint32_t n);
uint32_t reverse_bits(uint32_t n);
uint32_t reverse_bits_limited(uint32_t n, uint32_t value);
uint64_t log2_pow2(uint64_t n);
uint64_t reverse_bits(uint64_t n);
uint64_t reverse_bits_limited(uint64_t n, uint64_t value);
C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n);
void compute_powers(fr_t *out, const fr_t *x, uint64_t n);
void compute_powers(fr_t *out, const fr_t *x, size_t n);
bool pairings_verify(const g1_t *a1, const g2_t *a2, const g1_t *b1, const g2_t *b2);

#ifdef __cplusplus
Expand Down
4 changes: 2 additions & 2 deletions src/eip4844/blob.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
C_KZG_RET blob_to_polynomial(fr_t *p, const Blob *blob) {
C_KZG_RET ret;
for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
ret = bytes_to_bls_field(&p[i], (Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT]);
ret = bytes_to_bls_field(&p[i], (const Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT]);
if (ret != C_KZG_OK) return ret;
}
return C_KZG_OK;
Expand All @@ -42,7 +42,7 @@ C_KZG_RET blob_to_polynomial(fr_t *p, const Blob *blob) {
*/
void print_blob(const Blob *blob) {
for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
Bytes32 *field = (Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT];
const Bytes32 *field = (const Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT];
print_bytes32(field);
}
}
2 changes: 1 addition & 1 deletion src/eip7594/cell.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*/
void print_cell(const Cell *cell) {
for (size_t i = 0; i < FIELD_ELEMENTS_PER_CELL; i++) {
Bytes32 *field = (Bytes32 *)&cell->bytes[i * BYTES_PER_FIELD_ELEMENT];
const Bytes32 *field = (const Bytes32 *)&cell->bytes[i * BYTES_PER_FIELD_ELEMENT];
print_bytes32(field);
}
}
8 changes: 4 additions & 4 deletions src/eip7594/eip7594.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ C_KZG_RET recover_cells_and_kzg_proofs(

/* Convert the untrusted bytes to a field element */
size_t offset = j * BYTES_PER_FIELD_ELEMENT;
ret = bytes_to_bls_field(field, (Bytes32 *)&cells[i].bytes[offset]);
ret = bytes_to_bls_field(field, (const Bytes32 *)&cells[i].bytes[offset]);
if (ret != C_KZG_OK) goto out;
}
}
Expand Down Expand Up @@ -636,7 +636,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
for (size_t j = 0; j < FIELD_ELEMENTS_PER_CELL; j++) {
fr_t field, scaled;
size_t offset = j * BYTES_PER_FIELD_ELEMENT;
ret = bytes_to_bls_field(&field, (Bytes32 *)&cells[i].bytes[offset]);
ret = bytes_to_bls_field(&field, (const Bytes32 *)&cells[i].bytes[offset]);
if (ret != C_KZG_OK) goto out;
blst_fr_mul(&scaled, &field, &r_powers[i]);
size_t index = cell_indices[i] * FIELD_ELEMENTS_PER_CELL + j;
Expand Down Expand Up @@ -684,7 +684,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
* To unscale, divide by the coset. It's faster to multiply with the inverse. We can skip
* the first iteration because its dividing by one.
*/
uint32_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
uint64_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
fr_t inv_coset_factor;
blst_fr_eucl_inverse(&inv_coset_factor, &s->roots_of_unity[pos]);
shift_poly(column_interpolation_poly, FIELD_ELEMENTS_PER_CELL, &inv_coset_factor);
Expand Down Expand Up @@ -713,7 +713,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
////////////////////////////////////////////////////////////////////////////////////////////////

for (size_t i = 0; i < num_cells; i++) {
uint32_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, cell_indices[i]);
uint64_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, cell_indices[i]);
fr_t coset_factor = s->roots_of_unity[pos];
fr_pow(&weights[i], &coset_factor, FIELD_ELEMENTS_PER_CELL);
blst_fr_mul(&weighted_powers_of_r[i], &r_powers[i], &weights[i]);
Expand Down
4 changes: 2 additions & 2 deletions src/eip7594/fk20.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ C_KZG_RET compute_fk20_proofs(g1_t *out, const fr_t *p, size_t n, const KZGSetti
fr_t *toeplitz_coeffs_fft = NULL;
g1_t *h = NULL;
g1_t *h_ext_fft = NULL;
void *scratch = NULL;
limb_t *scratch = NULL;
bool precompute = s->wbits != 0;

/* Initialize length variables */
Expand All @@ -92,7 +92,7 @@ C_KZG_RET compute_fk20_proofs(g1_t *out, const fr_t *p, size_t n, const KZGSetti

if (precompute) {
/* Allocations for fixed-base MSM */
ret = c_kzg_malloc(&scratch, s->scratch_size);
ret = c_kzg_malloc((void **)&scratch, s->scratch_size);
if (ret != C_KZG_OK) goto out;
ret = c_kzg_calloc((void **)&scalars, FIELD_ELEMENTS_PER_CELL, sizeof(blst_scalar));
if (ret != C_KZG_OK) goto out;
Expand Down
1 change: 1 addition & 0 deletions src/eip7594/poly.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

#include "poly.h"
#include "common/alloc.h"
#include "common/ec.h"
#include "common/ret.h"
Expand Down
2 changes: 1 addition & 1 deletion src/eip7594/recovery.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ C_KZG_RET recover_cells(
/* Iterate over each cell index and check if we have received it */
if (!is_in_array(cell_indices, num_cells, i)) {
/* If the cell is missing, bit reverse the index and add it to the missing array */
uint32_t brp_i = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
uint64_t brp_i = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
missing_cell_indices[len_missing++] = brp_i;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/setup/setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static C_KZG_RET compute_roots_of_unity(KZGSettings *s) {
C_KZG_RET ret;
fr_t root_of_unity;

uint32_t max_scale = 0;
size_t max_scale = 0;
while ((1ULL << max_scale) < FIELD_ELEMENTS_PER_EXT_BLOB)
max_scale++;

Expand Down Expand Up @@ -382,7 +382,7 @@ C_KZG_RET load_trusted_setup(
}

/* 1<<max_scale is the smallest power of 2 >= n1 */
uint32_t max_scale = 0;
size_t max_scale = 0;
while ((1ULL << max_scale) < NUM_G1_POINTS)
max_scale++;

Expand Down
Loading
Loading