Disallow variable-length arrays and turn on -Wpedantic in Clang and GCC.

PiperOrigin-RevId: 562832747
Change-Id: I0532624cee31e089a47426305d885687eb0fc5c5
This commit is contained in:
Saran Tunyasuvunakool
2023-09-05 11:04:12 -07:00
committed by Copybara-Service
parent 8ffdee355a
commit 329ed193ac
17 changed files with 121 additions and 99 deletions
+6 -7
View File
@@ -88,8 +88,10 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
set(EXTRA_COMPILE_OPTIONS
-Werror
-Wall
-Wpedantic
-Wimplicit-fallthrough
-Wunused
-Wvla
-Wno-int-in-bool-context
-Wno-sign-compare
-Wno-unknown-pragmas
@@ -100,15 +102,12 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
-Wno-maybe-uninitialized
)
endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wgnu-empty-initializer)
endif()
endif()
if(WIN32)
add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
endif()
include(MujocoHarden)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} ${MUJOCO_HARDEN_COMPILE_OPTIONS})
set(EXTRA_LINK_OPTIONS ${EXTRA_LINK_OPTIONS} ${MUJOCO_HARDEN_LINK_OPTIONS})
if(WIN32)
add_definitions(-D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
endif()
+3 -3
View File
@@ -2318,7 +2318,7 @@ const char* mj_getPluginConfig(const mjModel* m, int plugin_id, const char* attr
void mj_loadPluginLibrary(const char* path);
void mj_loadAllPluginLibraries(const char* directory, mjfPluginLibraryLoadCallback callback);
int mj_version(void);
const char* mj_versionString();
const char* mj_versionString(void);
void mj_multiRay(const mjModel* m, mjData* d, const mjtNum pnt[3], const mjtNum* vec,
const mjtByte* geomgroup, mjtByte flg_static, int bodyexclude,
int* geomid, mjtNum* dist, int nray, mjtNum cutoff);
@@ -2575,12 +2575,12 @@ void mjd_quatIntegrate(const mjtNum vel[3], mjtNum scale,
mjtNum Dquat[9], mjtNum Dvel[9], mjtNum Dscale[3]);
void mjp_defaultPlugin(mjpPlugin* plugin);
int mjp_registerPlugin(const mjpPlugin* plugin);
int mjp_pluginCount();
int mjp_pluginCount(void);
const mjpPlugin* mjp_getPlugin(const char* name, int* slot);
const mjpPlugin* mjp_getPluginAtSlot(int slot);
void mjp_defaultResourceProvider(mjpResourceProvider* provider);
int mjp_registerResourceProvider(const mjpResourceProvider* provider);
int mjp_resourceProviderCount();
int mjp_resourceProviderCount(void);
const mjpResourceProvider* mjp_getResourceProvider(const char* resource_name);
const mjpResourceProvider* mjp_getResourceProviderAtSlot(int slot);
mjThreadPool* mju_threadPoolCreate(size_t number_of_threads);
+3 -3
View File
@@ -488,7 +488,7 @@ MJAPI void mj_loadAllPluginLibraries(const char* directory, mjfPluginLibraryLoad
MJAPI int mj_version(void);
// Return the current version of MuJoCo as a null-terminated string.
MJAPI const char* mj_versionString();
MJAPI const char* mj_versionString(void);
//---------------------------------- Ray collisions ------------------------------------------------
@@ -1278,7 +1278,7 @@ MJAPI void mjp_defaultPlugin(mjpPlugin* plugin);
MJAPI int mjp_registerPlugin(const mjpPlugin* plugin);
// Return the number of globally registered plugins.
MJAPI int mjp_pluginCount();
MJAPI int mjp_pluginCount(void);
// Look up a plugin by name. If slot is not NULL, also write its registered slot number into it.
MJAPI const mjpPlugin* mjp_getPlugin(const char* name, int* slot);
@@ -1295,7 +1295,7 @@ MJAPI void mjp_defaultResourceProvider(mjpResourceProvider* provider);
MJAPI int mjp_registerResourceProvider(const mjpResourceProvider* provider);
// Return the number of globally registered resource providers.
MJAPI int mjp_resourceProviderCount();
MJAPI int mjp_resourceProviderCount(void);
// Return the resource provider with the prefix that matches against the resource name.
// If no match, return NULL.
+6 -7
View File
@@ -88,8 +88,10 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
set(EXTRA_COMPILE_OPTIONS
-Werror
-Wall
-Wpedantic
-Wimplicit-fallthrough
-Wunused
-Wvla
-Wno-int-in-bool-context
-Wno-sign-compare
-Wno-unknown-pragmas
@@ -100,15 +102,12 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
-Wno-maybe-uninitialized
)
endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wgnu-empty-initializer)
endif()
endif()
if(WIN32)
add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
endif()
include(MujocoHarden)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} ${MUJOCO_HARDEN_COMPILE_OPTIONS})
set(EXTRA_LINK_OPTIONS ${EXTRA_LINK_OPTIONS} ${MUJOCO_HARDEN_LINK_OPTIONS})
if(WIN32)
add_definitions(-D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
endif()
+6 -7
View File
@@ -88,8 +88,10 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
set(EXTRA_COMPILE_OPTIONS
-Werror
-Wall
-Wpedantic
-Wimplicit-fallthrough
-Wunused
-Wvla
-Wno-int-in-bool-context
-Wno-sign-compare
-Wno-unknown-pragmas
@@ -100,15 +102,12 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang
-Wno-maybe-uninitialized
)
endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wgnu-empty-initializer)
endif()
endif()
if(WIN32)
add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
endif()
include(MujocoHarden)
set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} ${MUJOCO_HARDEN_COMPILE_OPTIONS})
set(EXTRA_LINK_OPTIONS ${EXTRA_LINK_OPTIONS} ${MUJOCO_HARDEN_LINK_OPTIONS})
if(WIN32)
add_definitions(-D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
endif()
+3 -1
View File
@@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
void _mj_crossplatform_void(void) {} // ISO C does not permit empty translation units
#if defined(__APPLE__) && defined(__AVX__)
#include <stdio.h>
@@ -23,7 +25,7 @@ __attribute__((weak, visibility("default"))) void _mj_rosettaError(const char* m
__asm__ __volatile__ ("ud2"); // raises SIGILL but leave this function at the top of the stack
}
__attribute__((constructor(10000), target("no-avx"))) static void _mj_checkRosetta() {
__attribute__((constructor(10000), target("no-avx"))) static void _mj_checkRosetta(void) {
int is_translated = 0;
{
size_t len = sizeof(is_translated);
+2 -2
View File
@@ -32,10 +32,10 @@ MJAPI int mjp_registerPlugin(const mjpPlugin* plugin);
MJAPI int mjp_registerResourceProvider(const mjpResourceProvider* provider);
// return the number of globally registered plugins
MJAPI int mjp_pluginCount();
MJAPI int mjp_pluginCount(void);
// return the number of globally registered resource providers
MJAPI int mjp_resourceProviderCount();
MJAPI int mjp_resourceProviderCount(void);
// look up a plugin by name, optionally also get its registered slot number
MJAPI const mjpPlugin* mjp_getPlugin(const char* name, int* slot);
+6 -3
View File
@@ -14,10 +14,13 @@
#include "engine/engine_support.h"
#include <stddef.h>
#include <stdint.h>
#include <string.h>
#include <mujoco/mjdata.h>
#include <mujoco/mjmacro.h>
#include <mujoco/mjmodel.h>
#include "engine/engine_array_safety.h"
#include "engine/engine_core_constraint.h"
#include "engine/engine_crossplatform.h"
#include "engine/engine_io.h"
@@ -1056,7 +1059,7 @@ void mj_makeMSparse(const mjModel* m, mjData* d, mjtNum* M,
}
// backward pass over dofs: construct M_row(i) in reverse order
int col = M_rowadr[i]; // current column in row i
int col = M_rowadr[i]; // current column in row i
for (int j = i; j >= 0; j = m->dof_parentid[j]) {
M[col] = d->qM[Madr++];
M_colind[col++] = j;
@@ -1584,7 +1587,7 @@ int mj_version(void) {
// current version of MuJoCo as a null-terminated string
const char* mj_versionString() {
const char* mj_versionString(void) {
static const char versionstring[] = mjVERSIONSTRING;
return versionstring;
}
+4 -1
View File
@@ -15,9 +15,12 @@
#ifndef MUJOCO_SRC_ENGINE_ENGINE_SUPPORT_H_
#define MUJOCO_SRC_ENGINE_ENGINE_SUPPORT_H_
#include <stdint.h>
#include <mujoco/mjdata.h>
#include <mujoco/mjexport.h>
#include <mujoco/mjmodel.h>
#include <mujoco/mjtnum.h>
#ifdef __cplusplus
extern "C" {
@@ -197,7 +200,7 @@ MJAPI void mj_warning(mjData* d, int warning, int info);
MJAPI int mj_version(void);
// current version of MuJoCo as a null-terminated string
MJAPI const char* mj_versionString();
MJAPI const char* mj_versionString(void);
#ifdef __cplusplus
}
#endif
+18 -11
View File
@@ -70,7 +70,7 @@ typedef void (*callback_fn)(const char*);
static mjTHREADLOCAL callback_fn _mjPRIVATE_tls_error_fn = NULL;
static mjTHREADLOCAL callback_fn _mjPRIVATE_tls_warning_fn = NULL;
callback_fn _mjPRIVATE__get_tls_error_fn() {
callback_fn _mjPRIVATE__get_tls_error_fn(void) {
return _mjPRIVATE_tls_error_fn;
}
@@ -78,7 +78,7 @@ void _mjPRIVATE__set_tls_error_fn(callback_fn h) {
_mjPRIVATE_tls_error_fn = h;
}
callback_fn _mjPRIVATE__get_tls_warning_fn() {
callback_fn _mjPRIVATE__get_tls_warning_fn(void) {
return _mjPRIVATE_tls_warning_fn;
}
@@ -113,20 +113,17 @@ void mju_writeLog(const char* type, const char* msg) {
}
}
void mju_error_v(const char* msg, va_list args) {
char errmsg[1000];
// Format msg into errmsg
vsnprintf(errmsg, mjSIZEOFARRAY(errmsg), msg, args);
void mju_error_raw(const char* msg) {
if (_mjPRIVATE_tls_error_fn) {
_mjPRIVATE_tls_error_fn(errmsg);
_mjPRIVATE_tls_error_fn(msg);
} else if (mju_user_error) {
mju_user_error(errmsg);
mju_user_error(msg);
} else {
// write to log and console
mju_writeLog("ERROR", errmsg);
printf("ERROR: %s\n\nPress Enter to exit ...", errmsg);
mju_writeLog("ERROR", msg);
printf("ERROR: %s\n\nPress Enter to exit ...", msg);
// pause, exit
getchar();
@@ -135,6 +132,16 @@ void mju_error_v(const char* msg, va_list args) {
}
void mju_error_v(const char* msg, va_list args) {
// Format msg into errmsg
char errmsg[1024];
vsnprintf(errmsg, mjSIZEOFARRAY(errmsg), msg, args);
mju_error_raw(errmsg);
}
// write message to logfile and console, pause and exit
void mju_error(const char* msg, ...) {
va_list args;
@@ -147,7 +154,7 @@ void mju_error(const char* msg, ...) {
// write message to logfile and console
void mju_warning(const char* msg, ...) {
char wrnmsg[1000];
char wrnmsg[1024];
// Format msg into wrnmsg
va_list args;
+13 -38
View File
@@ -18,6 +18,7 @@
#include <stdarg.h>
#include <stddef.h>
#include <stdio.h>
#include <string.h>
#include <mujoco/mjexport.h>
#include <mujoco/mjmacro.h>
@@ -31,8 +32,8 @@ extern "C" {
#define mjPRINTFLIKE(n, m) __attribute__((format(printf, n, m)))
#else
#define mjPRINTFLIKE(n, m)
#endif // __GNUC__
#endif // mjPRINTFLIKE
#endif // __GNUC__
#endif // mjPRINTFLIKE
//------------------------------ user handlers -----------------------------------------------------
@@ -54,6 +55,7 @@ MJAPI void _mjPRIVATE__set_tls_warning_fn(void (*h)(const char*));
//------------------------------ errors and warnings -----------------------------------------------
// errors
MJAPI void mju_error_raw(const char* msg);
MJAPI void mju_error(const char* msg, ...) mjPRINTFLIKE(1, 2);
MJAPI void mju_error_v(const char* msg, va_list args);
MJAPI void mju_error_i(const char* msg, int i);
@@ -69,42 +71,15 @@ MJAPI void mju_writeLog(const char* type, const char* msg);
//------------------------------ internal error macros --------------------------------------------
// need at least c99 or c++11
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) || \
(defined(__cplusplus) && __cplusplus >= 201103L)
// macro to get the first argument
#define _GET_MSG(msg, ...) msg
// helper function for the mjERROR macro
// formats buf as '{prefix}: {msg}' and passes along to mju_error_v
static inline void _mju_error_prefix(char *buf, size_t nbuf, const char* prefix,
const char* msg, ...) mjPRINTFLIKE(4, 5);
static inline void _mju_error_prefix(char *buf, size_t nbuf, const char* prefix,
const char* msg, ...) {
snprintf(buf, nbuf, "%s: %s", prefix, msg);
va_list args;
va_start(args, msg);
mju_error_v(buf, args);
va_end(args);
}
// macro to get first argument
#define _GET_MSG(msg, ...) msg
// internal macro to prepend the calling function name to the error message
// standard support for variadic macros with zero arguments is only now
// supported in C23 and C++20 so we rely on a helper function to get around this
// in a portable way
#define mjERROR(...) { \
char _buf[sizeof(_GET_MSG(__VA_ARGS__)) + sizeof(__func__) + 1]; \
_mju_error_prefix(_buf, sizeof(_buf), __func__, __VA_ARGS__); \
}
#else
#define mjERROR mju_error
#endif // c99 or c++11
// internal macro to prepend the calling function name to the error message
#define mjERROR(...) \
{ \
char _errbuf[1024]; \
size_t _funclen = strlen(__func__); \
strncpy(_errbuf, __func__, sizeof(_errbuf)); \
snprintf(_errbuf + _funclen, sizeof(_errbuf) - _funclen, ": " __VA_ARGS__); \
mju_error_raw(_errbuf); \
}
//------------------------------ malloc and free ---------------------------------------------------
+13 -4
View File
@@ -36,6 +36,11 @@
// Online:
// https://glad.dav1d.de/#profile=compatibility&language=c&specification=gl&loader=on&api=gl%3D1.5&extensions=GL_ARB_framebuffer_object&extensions=GL_ARB_seamless_cube_map&extensions=GL_ARB_vertex_buffer_object&extensions=GL_KHR_debug
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpedantic"
#endif
#if !defined(_WIN32) && !defined(__CYGWIN__) && !defined(__APPLE__) && \
!defined(__HAIKU__) && !defined(_GNU_SOURCE)
#define _GNU_SOURCE
@@ -1471,12 +1476,12 @@ static void mjGlad_find_coreGL(void) {
}
}
int mjGladLoadGLUnsafe() {
if(mjGlad_open_gl()) {
int mjGladLoadGLUnsafe(void) {
if (mjGlad_open_gl()) {
mjGLVersion.major = 0; mjGLVersion.minor = 0;
glGetString = (PFNGLGETSTRINGPROC)mjGlad_get_proc("glGetString");
if(glGetString == NULL) return 0;
if(glGetString(GL_VERSION) == NULL) return 0;
if (glGetString == NULL) return 0;
if (glGetString(GL_VERSION) == NULL) return 0;
mjGlad_find_coreGL();
mjGlad_load_GL_VERSION_1_0(mjGlad_get_proc);
mjGlad_load_GL_VERSION_1_1(mjGlad_get_proc);
@@ -1496,3 +1501,7 @@ int mjGladLoadGLUnsafe() {
return 0;
}
}
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
+2 -2
View File
@@ -140,7 +140,7 @@ static void makePlane(const mjModel* m, mjrContext* con) {
// record
grid[k][x] = left;
grid[k][x+1] = mjMAX(left, right); // just in case
grid[k][x+1] = mjMAX(left, right); // just in case
}
}
@@ -1458,7 +1458,7 @@ void GLAPIENTRY debugCallback(GLenum source,
// returns 1 if MUJOCO_GL_DEBUG environment variable is set to 1
static int glDebugEnabled() {
static int glDebugEnabled(void) {
char* debug = getenv("MUJOCO_GL_DEBUG");
return debug && strcmp(debug, "1") == 0;
}
+19 -2
View File
@@ -13,12 +13,14 @@
// limitations under the License.
#include <algorithm>
#include <array>
#include <cmath>
#include <csetjmp>
#include <cstddef>
#include <cstdio>
#include <cstring>
#include <memory>
#include <optional>
#include <string>
#include <utility>
#include <vector>
@@ -27,11 +29,27 @@
#define TINYOBJLOADER_IMPLEMENTATION
#endif
#if defined(__clang__)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wgnu-anonymous-struct"
#pragma clang diagnostic ignored "-Wnested-anon-types"
#elif defined(__GNUC__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpedantic"
#endif
#include <MC.h>
#if defined(__clang__)
#pragma clang diagnostic pop
#elif defined(__GNUC__)
#pragma GCC diagnostic pop
#endif
#include <mujoco/mjmacro.h>
#include <mujoco/mjmodel.h>
#include <mujoco/mjtnum.h>
#include <mujoco/mjplugin.h>
#include "cc/array_safety.h"
#include "engine/engine_crossplatform.h"
#include "engine/engine_plugin.h"
#include "engine/engine_resource.h"
#include "engine/engine_util_blas.h"
#include "engine/engine_util_errmem.h"
@@ -1838,7 +1856,6 @@ mjCSkin::~mjCSkin() {
// compiler
void mjCSkin::Compile(const mjVFS* vfs) {
// load file
if (!file.empty()) {
// make sure data is not present
+4 -7
View File
@@ -14,22 +14,22 @@
#include "user/user_objects.h"
#include <algorithm>
#include <cmath>
#include <cstddef>
#include <cstdlib>
#include <cstring>
#include <iostream>
#include <sstream>
#include <string>
#include <string_view>
#include <vector>
#include "lodepng.h"
#include <mujoco/mjmacro.h>
#include <mujoco/mjmodel.h>
#include <mujoco/mjplugin.h>
#include <mujoco/mjtnum.h>
#include "cc/array_safety.h"
#include "engine/engine_core_smooth.h"
#include "engine/engine_crossplatform.h"
#include "engine/engine_resource.h"
#include "engine/engine_io.h"
#include "engine/engine_passive.h"
@@ -131,7 +131,7 @@ mjCError::mjCError(const mjCBase* obj, const char* msg, const char* str, int pos
// constructor
mjCAlternative::mjCAlternative() {
axisangle[0] = xyaxes[0] = zaxis[0] = euler[0] = fullinertia[0] = mjNAN;
};
}
// compute frame orientation given alternative specifications
@@ -1514,7 +1514,6 @@ void mjCGeom::SetFluidCoefs(void) {
// get semiaxes
switch (type) {
case mjGEOM_SPHERE:
dx = size[0];
dy = size[0];
@@ -4497,7 +4496,6 @@ mjCKey::~mjCKey() {
// compiler
void mjCKey::Compile(const mjModel* m) {
// qpos: allocate or check size
if (qpos.empty()) {
qpos.resize(m->nq);
@@ -4572,7 +4570,6 @@ void mjCKey::Compile(const mjModel* m) {
} else if (ctrl.size()!=m->nu) {
throw mjCError(this, "key %d: invalid ctrl size, expected length %d", nullptr, id, m->nu);
}
}
+9 -1
View File
@@ -23,7 +23,7 @@
namespace mujoco {
namespace {
constexpr int kBufferSize = 1000;
constexpr int kBufferSize = 1024;
char* ErrorMessageBuffer() {
static char error_message[kBufferSize] = "";
@@ -132,5 +132,13 @@ TEST_F(MujocoErrorAndWarningTest, MjuWarningS) {
EXPECT_EQ(std::string(WarningMessageBuffer()), expected_message);
}
TEST_F(MujocoErrorAndWarningTest, MjuErrorInternal) {
ClearErrorMessage();
mjERROR("foobar %d", 123);
std::string funcname(__func__);
ASSERT_TRUE(funcname.length());
EXPECT_EQ(std::string(ErrorMessageBuffer()), funcname + ": foobar 123");
}
} // namespace
} // namespace mujoco
+4
View File
@@ -3369,6 +3369,10 @@ public static unsafe extern void mj_loadPluginLibrary([MarshalAs(UnmanagedType.L
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
public static unsafe extern int mj_version();
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
[return: MarshalAs(UnmanagedType.LPStr)]
public static unsafe extern string mj_versionString();
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
public static unsafe extern void mj_multiRay(mjModel_* m, mjData_* d, double* pnt, double* vec, byte* geomgroup, byte flg_static, int bodyexclude, int* geomid, double* dist, int nray, double cutoff);