diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 95c33dd3..a3e33f38 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,11 +10,9 @@ name: build on: push: - branches: [main] paths-ignore: - "doc/**" pull_request: - branches: [main] paths-ignore: - "doc/**" @@ -115,9 +113,9 @@ jobs: - name: Prepare macOS if: ${{ runner.os == 'macOS' }} run: brew install ninja - - uses: actions/setup-python@v3 + - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: Prepare Python shell: bash run: | diff --git a/cmake/MujocoOptions.cmake b/cmake/MujocoOptions.cmake index 9abbd8d2..62e15012 100644 --- a/cmake/MujocoOptions.cmake +++ b/cmake/MujocoOptions.cmake @@ -85,15 +85,19 @@ include(MujocoLinkOptions) get_mujoco_extra_link_options(EXTRA_LINK_OPTIONS) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)) - set(EXTRA_COMPILE_OPTIONS -Wall -Werror) + set(EXTRA_COMPILE_OPTIONS + -Werror + -Wall + -Wimplicit-fallthrough + -Wunused + -Wno-int-in-bool-context + -Wno-sign-compare + -Wno-unknown-pragmas + ) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - # TODO: This should add to EXTRA_COMPILE_OPTIONS rather than overwrite it. - set(EXTRA_COMPILE_OPTIONS - -Wno-int-in-bool-context - -Wno-maybe-uninitialized - -Wno-sign-compare - -Wno-stringop-overflow - -Wno-stringop-truncation + # Set -Wimplicit-fallthrough=5 to only allow fallthrough annotation via __attribute__. + set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wimplicit-fallthrough=5 + -Wno-maybe-uninitialized ) endif() endif() diff --git a/plugin/elasticity/CMakeLists.txt b/plugin/elasticity/CMakeLists.txt index b16cd3e8..f35cf2d3 100644 --- a/plugin/elasticity/CMakeLists.txt +++ b/plugin/elasticity/CMakeLists.txt @@ -12,9 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -set(MUJOCO_ELASTICITY_INCLUDE - ${CMAKE_CURRENT_SOURCE_DIR}/../.. - ${CMAKE_CURRENT_SOURCE_DIR}/../../src +set(MUJOCO_ELASTICITY_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/../.. + ${CMAKE_CURRENT_SOURCE_DIR}/../../src ) set(MUJOCO_ELASTICITY_SRCS diff --git a/python/mujoco/CMakeLists.txt b/python/mujoco/CMakeLists.txt index 9160f4cc..dce3cca7 100644 --- a/python/mujoco/CMakeLists.txt +++ b/python/mujoco/CMakeLists.txt @@ -67,7 +67,12 @@ include(FindOrFetch) # ==================== MUJOCO LIBRARY ========================================== if(NOT TARGET mujoco) - add_library(mujoco SHARED IMPORTED GLOBAL) + add_library( + mujoco + SHARED + IMPORTED + GLOBAL + ) if(APPLE) # On macOS, check if we are using mujoco.framework first. # Framework headers are searched differently from normal headers. @@ -75,13 +80,17 @@ if(NOT TARGET mujoco) find_path(MUJOCO_FRAMEWORK mujoco.Framework HINTS ${MUJOCO_FRAMEWORK_DIR}) if(MUJOCO_FRAMEWORK) message("MuJoCo framework is at ${MUJOCO_FRAMEWORK}/mujoco.framework") - set(MUJOCO_LIBRARY ${MUJOCO_FRAMEWORK}/mujoco.framework/Versions/A/libmujoco.2.3.2.dylib) + set(MUJOCO_LIBRARY + ${MUJOCO_FRAMEWORK}/mujoco.framework/Versions/A/libmujoco.2.3.2.dylib + ) target_compile_options(mujoco INTERFACE -F${MUJOCO_FRAMEWORK}) endif() endif() if(NOT MUJOCO_FRAMEWORK) - find_library(MUJOCO_LIBRARY mujoco mujoco.2.3.2 HINTS ${MUJOCO_LIBRARY_DIR} REQUIRED) + find_library( + MUJOCO_LIBRARY mujoco mujoco.2.3.2 HINTS ${MUJOCO_LIBRARY_DIR} REQUIRED + ) find_path(MUJOCO_INCLUDE mujoco/mujoco.h HINTS ${MUJOCO_INCLUDE_DIR} REQUIRED) message("MuJoCo is at ${MUJOCO_LIBRARY}") message("MuJoCo headers are at ${MUJOCO_INCLUDE}") @@ -276,16 +285,18 @@ macro(mujoco_pybind11_module name) pybind11_add_module(${name} ${ARGN}) target_compile_options(${name} PRIVATE ${AVX_COMPILE_OPTIONS}) if(NOT MSVC) - target_compile_options(${name} PRIVATE -Wall -Werror) - if(CMAKE_C_COMPILER_ID STREQUAL GNU) - target_compile_options( - ${name} - PRIVATE -Wno-int-in-bool-context - -Wno-maybe-uninitialized - -Wno-sign-compare - -Wno-stringop-overflow - -Wno-stringop-truncation - ) + target_compile_options( + ${name} + PRIVATE -Werror + -Wall + -Wimplicit-fallthrough + -Wunused + -Wno-int-in-bool-context + -Wno-sign-compare + ) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # Only allow fallthrough annotation via __attribute__. + target_compile_options(${name} PRIVATE -Wimplicit-fallthrough=5) endif() endif() set_target_properties(${name} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) @@ -380,7 +391,8 @@ target_link_libraries( PRIVATE mujoco mujoco::libsimulate raw - structs_header) + structs_header +) set(LIBRARIES_FOR_WHEEL "$" diff --git a/python/mujoco/errors.h b/python/mujoco/errors.h index 7475a0e1..2febacd8 100644 --- a/python/mujoco/errors.h +++ b/python/mujoco/errors.h @@ -110,7 +110,8 @@ static thread_local std::jmp_buf mju_error_jmp_buf; static thread_local std::array mju_error_msg{0}; static inline void MjErrorHandler(const char* msg) { - std::strncpy(mju_error_msg.data(), msg, mju_error_msg.size()); + std::strncpy(mju_error_msg.data(), msg, mju_error_msg.size() - 1); + mju_error_msg.data()[mju_error_msg.size() - 1] = '\0'; std::longjmp(mju_error_jmp_buf, 1); } diff --git a/python/mujoco/simulate.cc b/python/mujoco/simulate.cc index 8f778a6c..df7e42e6 100644 --- a/python/mujoco/simulate.cc +++ b/python/mujoco/simulate.cc @@ -25,6 +25,11 @@ namespace mujoco::python { namespace { +template +constexpr inline std::size_t sizeof_arr(const T(&arr)[N]) { + return sizeof(arr); +} + PYBIND11_MODULE(_simulate, pymodule) { namespace py = ::pybind11; using SimulateMutex = decltype(mujoco::Simulate::mtx); @@ -138,8 +143,9 @@ PYBIND11_MODULE(_simulate, pymodule) { return simulate.loadError; }, [](mujoco::Simulate& simulate, const std::string& error) { - std::strncpy(simulate.loadError, error.c_str(), - simulate.kMaxFilenameLength); + const auto max_length = sizeof_arr(simulate.loadError); + std::strncpy(simulate.loadError, error.c_str(), max_length - 1); + simulate.loadError[max_length - 1] = '\0'; }); pymodule.def("setglfwdlhandle", [](std::uintptr_t dlhandle) { diff --git a/sample/array_safety.h b/sample/array_safety.h index 19881f8a..e8dd0d73 100644 --- a/sample/array_safety.h +++ b/sample/array_safety.h @@ -73,7 +73,16 @@ static inline int sprintf_arr(char (&dest)[N], const char* format, ...) { // dest is guaranteed to be null-terminated template static inline char* strcat_arr(char (&dest)[N], const char* src) { - return std::strncat(dest, src, sizeof_arr(dest) - strlen_arr(dest) - 1); + const std::size_t dest_len = strlen_arr(dest); + const std::size_t dest_size = sizeof_arr(dest); + for (std::size_t i = dest_len; i < dest_size; ++i) { + dest[i] = src[i - dest_len]; + if (!dest[i]) { + break; + } + } + dest[dest_size - 1] = '\0'; + return dest; } // like std::strcpy but won't write beyond the bound of dest diff --git a/sample/cmake/SampleOptions.cmake b/sample/cmake/SampleOptions.cmake index 9abbd8d2..62e15012 100644 --- a/sample/cmake/SampleOptions.cmake +++ b/sample/cmake/SampleOptions.cmake @@ -85,15 +85,19 @@ include(MujocoLinkOptions) get_mujoco_extra_link_options(EXTRA_LINK_OPTIONS) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)) - set(EXTRA_COMPILE_OPTIONS -Wall -Werror) + set(EXTRA_COMPILE_OPTIONS + -Werror + -Wall + -Wimplicit-fallthrough + -Wunused + -Wno-int-in-bool-context + -Wno-sign-compare + -Wno-unknown-pragmas + ) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - # TODO: This should add to EXTRA_COMPILE_OPTIONS rather than overwrite it. - set(EXTRA_COMPILE_OPTIONS - -Wno-int-in-bool-context - -Wno-maybe-uninitialized - -Wno-sign-compare - -Wno-stringop-overflow - -Wno-stringop-truncation + # Set -Wimplicit-fallthrough=5 to only allow fallthrough annotation via __attribute__. + set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wimplicit-fallthrough=5 + -Wno-maybe-uninitialized ) endif() endif() diff --git a/simulate/CMakeLists.txt b/simulate/CMakeLists.txt index c120a1f0..af8f2ad4 100644 --- a/simulate/CMakeLists.txt +++ b/simulate/CMakeLists.txt @@ -104,19 +104,13 @@ endif() add_library(platform_ui_adapter OBJECT) target_sources( platform_ui_adapter - PUBLIC glfw_adapter.h - glfw_dispatch.h - platform_ui_adapter.h - PRIVATE glfw_adapter.cc - glfw_dispatch.cc - platform_ui_adapter.cc + PUBLIC glfw_adapter.h glfw_dispatch.h platform_ui_adapter.h + PRIVATE glfw_adapter.cc glfw_dispatch.cc platform_ui_adapter.cc ) target_compile_options(platform_ui_adapter PUBLIC ${MUJOCO_SIMULATE_COMPILE_OPTIONS}) target_include_directories( - platform_ui_adapter - PUBLIC - ${CMAKE_CURRENT_SOURCE_DIR} - $ + platform_ui_adapter PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} + $ ) target_link_libraries(platform_ui_adapter PUBLIC mujoco::mujoco) if(SIMULATE_GLFW_DYNAMIC_SYMBOLS) @@ -131,8 +125,7 @@ add_library(mujoco::libsimulate ALIAS libsimulate) target_sources( libsimulate PUBLIC simulate.h - PRIVATE simulate.cc - array_safety.h + PRIVATE simulate.cc array_safety.h ) target_include_directories(libsimulate PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) target_compile_options(libsimulate PUBLIC ${MUJOCO_SIMULATE_COMPILE_OPTIONS}) diff --git a/simulate/array_safety.h b/simulate/array_safety.h index 19881f8a..e8dd0d73 100644 --- a/simulate/array_safety.h +++ b/simulate/array_safety.h @@ -73,7 +73,16 @@ static inline int sprintf_arr(char (&dest)[N], const char* format, ...) { // dest is guaranteed to be null-terminated template static inline char* strcat_arr(char (&dest)[N], const char* src) { - return std::strncat(dest, src, sizeof_arr(dest) - strlen_arr(dest) - 1); + const std::size_t dest_len = strlen_arr(dest); + const std::size_t dest_size = sizeof_arr(dest); + for (std::size_t i = dest_len; i < dest_size; ++i) { + dest[i] = src[i - dest_len]; + if (!dest[i]) { + break; + } + } + dest[dest_size - 1] = '\0'; + return dest; } // like std::strcpy but won't write beyond the bound of dest diff --git a/simulate/cmake/SimulateOptions.cmake b/simulate/cmake/SimulateOptions.cmake index 9abbd8d2..62e15012 100644 --- a/simulate/cmake/SimulateOptions.cmake +++ b/simulate/cmake/SimulateOptions.cmake @@ -85,15 +85,19 @@ include(MujocoLinkOptions) get_mujoco_extra_link_options(EXTRA_LINK_OPTIONS) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT MSVC)) - set(EXTRA_COMPILE_OPTIONS -Wall -Werror) + set(EXTRA_COMPILE_OPTIONS + -Werror + -Wall + -Wimplicit-fallthrough + -Wunused + -Wno-int-in-bool-context + -Wno-sign-compare + -Wno-unknown-pragmas + ) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - # TODO: This should add to EXTRA_COMPILE_OPTIONS rather than overwrite it. - set(EXTRA_COMPILE_OPTIONS - -Wno-int-in-bool-context - -Wno-maybe-uninitialized - -Wno-sign-compare - -Wno-stringop-overflow - -Wno-stringop-truncation + # Set -Wimplicit-fallthrough=5 to only allow fallthrough annotation via __attribute__. + set(EXTRA_COMPILE_OPTIONS ${EXTRA_COMPILE_OPTIONS} -Wimplicit-fallthrough=5 + -Wno-maybe-uninitialized ) endif() endif() diff --git a/src/cc/array_safety.h b/src/cc/array_safety.h index 8951c32d..6782c03a 100644 --- a/src/cc/array_safety.h +++ b/src/cc/array_safety.h @@ -73,7 +73,16 @@ static inline int sprintf_arr(char (&dest)[N], const char* format, ...) { // dest is guaranteed to be null-terminated template static inline char* strcat_arr(char (&dest)[N], const char* src) { - return std::strncat(dest, src, sizeof_arr(dest) - strlen_arr(dest) - 1); + const std::size_t dest_len = strlen_arr(dest); + const std::size_t dest_size = sizeof_arr(dest); + for (std::size_t i = dest_len; i < dest_size; ++i) { + dest[i] = src[i - dest_len]; + if (!dest[i]) { + break; + } + } + dest[dest_size - 1] = '\0'; + return dest; } // like std::strcpy but won't write beyond the bound of dest diff --git a/src/engine/engine_collision_box.c b/src/engine/engine_collision_box.c index c8dcc0e5..fae00bc6 100644 --- a/src/engine/engine_collision_box.c +++ b/src/engine/engine_collision_box.c @@ -316,7 +316,7 @@ int mjc_CapsuleBox(const mjModel* m, const mjData* d, mjContact* con, }; mjtNum c[2]; } d2; - d2 p, s, d, c /*, tmp1*/; + d2 p, s, d /*, c, tmp1*/; mjtNum u, v, w, e1, best /* ,e2 */, l /* , e3, e4 */; bestdist = bestdistmax; @@ -366,8 +366,8 @@ int mjc_CapsuleBox(const mjModel* m, const mjData* d, mjContact* con, } } - c.x = s.x * ((c1 / 2) ? -1 : 1); - c.y = s.y * ((c1 % 2) ? -1 : 1); + // c.x = s.x * ((c1 / 2) ? -1 : 1); + // c.y = s.y * ((c1 % 2) ? -1 : 1); e1 = fabs(w) / l; // e2 = best / l; diff --git a/src/engine/engine_core_constraint.c b/src/engine/engine_core_constraint.c index e60ab3f4..492763d8 100644 --- a/src/engine/engine_core_constraint.c +++ b/src/engine/engine_core_constraint.c @@ -540,6 +540,7 @@ void mj_instantiateEquality(const mjModel* m, mjData* d) { case mjEQ_DISTANCE: mju_error("distance equality constraints are no longer supported"); + break; default: // SHOULD NOT OCCUR mju_error_i("Invalid equality constraint type %d", m->eq_type[i]); diff --git a/src/engine/engine_core_smooth.c b/src/engine/engine_core_smooth.c index 66a8863e..9fda1bab 100644 --- a/src/engine/engine_core_smooth.c +++ b/src/engine/engine_core_smooth.c @@ -21,6 +21,7 @@ #include #include "engine/engine_callback.h" #include "engine/engine_core_constraint.h" +#include "engine/engine_crossplatform.h" #include "engine/engine_io.h" #include "engine/engine_macro.h" #include "engine/engine_plugin.h" @@ -236,6 +237,7 @@ void mj_comPos(const mjModel* m, mjData* d) { // rotation components: same as ball skip = 18; + mjFALLTHROUGH; case mjJNT_BALL: for (int i=0; i<3; i++) { @@ -1283,6 +1285,7 @@ void mj_comVel(const mjModel* m, mjData* d) { // continue with rotations j += 3; + mjFALLTHROUGH; case mjJNT_BALL: // compute all 3 cdofdots using parent velocity @@ -1349,6 +1352,7 @@ void mj_passive(const mjModel* m, mjData* d) { // continue with rotations dadr += 3; padr += 3; + mjFALLTHROUGH; case mjJNT_BALL: // covert quatertion difference into angular "velocity" @@ -2032,6 +2036,7 @@ void mj_rnePostConstraint(const mjModel* m, mjData* d) { case mjEQ_DISTANCE: mju_error("distance equality constraints are no longer supported"); + break; default: mju_error_i("Unknown constraint type type %d", m->eq_type[id]); // SHOULD NOT OCCUR diff --git a/src/engine/engine_crossplatform.h b/src/engine/engine_crossplatform.h index 3728516a..05c08ab7 100644 --- a/src/engine/engine_crossplatform.h +++ b/src/engine/engine_crossplatform.h @@ -46,8 +46,10 @@ #endif #endif -#if defined(__GNUC__) && __GNUC__ >= 7 - #define mjFALLTHROUGH __attribute__ ((fallthrough)) +#if defined(__cplusplus) + #define mjFALLTHROUGH [[fallthrough]] +#elif defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 7) + #define mjFALLTHROUGH __attribute__((fallthrough)) #else #define mjFALLTHROUGH ((void) 0) #endif diff --git a/src/engine/engine_derivative.c b/src/engine/engine_derivative.c index 082de42c..4d1dca9c 100644 --- a/src/engine/engine_derivative.c +++ b/src/engine/engine_derivative.c @@ -22,6 +22,7 @@ #include "engine/engine_core_smooth.h" #include "engine/engine_forward.h" #include "engine/engine_core_constraint.h" +#include "engine/engine_crossplatform.h" #include "engine/engine_io.h" #include "engine/engine_inverse.h" #include "engine/engine_macro.h" @@ -402,6 +403,7 @@ static void mjd_comVel_vel(const mjModel* m, mjData* d, mjtNum* Dcvel, mjtNum* D // continue with rotations j += 3; + mjFALLTHROUGH; case mjJNT_BALL: // Dcdofdot = D crossMotion(cvel, cdof) diff --git a/src/engine/engine_sensor.c b/src/engine/engine_sensor.c index 87f8db35..9f2cbf44 100644 --- a/src/engine/engine_sensor.c +++ b/src/engine/engine_sensor.c @@ -21,6 +21,7 @@ #include #include "engine/engine_callback.h" #include "engine/engine_core_smooth.h" +#include "engine/engine_crossplatform.h" #include "engine/engine_io.h" #include "engine/engine_macro.h" #include "engine/engine_plugin.h" @@ -804,6 +805,7 @@ void mj_energyPos(const mjModel* m, mjData* d) { // continue with rotations padr += 3; + mjFALLTHROUGH; case mjJNT_BALL: // covert quatertion difference into angular "velocity" diff --git a/src/engine/engine_support.c b/src/engine/engine_support.c index e794116d..89289e0c 100644 --- a/src/engine/engine_support.c +++ b/src/engine/engine_support.c @@ -1270,7 +1270,8 @@ void mj_differentiatePos(const mjModel* m, mjtNum* qvel, mjtNum dt, vadr += 3; padr += 3; - // continute with rotations + // continute with rotations + mjFALLTHROUGH; case mjJNT_BALL: mju_negQuat(neg, qpos1+padr); // solve: qpos1 * dif = qpos2 @@ -1304,7 +1305,8 @@ void mj_integratePos(const mjModel* m, mjtNum* qpos, const mjtNum* qvel, mjtNum padr += 3; vadr += 3; - // continue with rotation update + // continue with rotation update + mjFALLTHROUGH; case mjJNT_BALL: // quaternion update diff --git a/src/engine/engine_vis_visualize.c b/src/engine/engine_vis_visualize.c index c61c9b17..1c071ed8 100644 --- a/src/engine/engine_vis_visualize.c +++ b/src/engine/engine_vis_visualize.c @@ -62,6 +62,7 @@ static void makeLabel(const mjModel* m, mjtObj type, int id, char* label) { // copy result into label strncpy(label, txt, 99); + label[99] = '\0'; } diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index 0e50c231..ce9d2f11 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -1854,9 +1854,9 @@ void mjCSkin::LoadSKN(const mjVFS* vfs) { // read name char txt[40]; - strncpy(txt, (char*)(pdata+cnt), 40); + strncpy(txt, (char*)(pdata+cnt), 39); + txt[39] = '\0'; cnt += 10; - txt[39] = 0; bodyname.push_back(txt); // read bindpos diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6c64c8ab..6bec9110 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -21,7 +21,13 @@ macro(mujoco_test name) set(options) set(oneValueArgs) set(multiValueArgs PROPERTIES) - cmake_parse_arguments(_ARGS "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) + cmake_parse_arguments( + _ARGS + "${options}" + "${oneValueArgs}" + "${multiValueArgs}" + ${ARGN} + ) add_executable(${name} ${name}.cc) target_link_libraries(${name} gtest_main mujoco) diff --git a/test/user/CMakeLists.txt b/test/user/CMakeLists.txt index d647f06a..d31037b1 100644 --- a/test/user/CMakeLists.txt +++ b/test/user/CMakeLists.txt @@ -19,7 +19,12 @@ mujoco_test(user_objects_test) target_link_libraries(user_objects_test fixture gmock) mujoco_test(user_mesh_test) -target_link_libraries(user_mesh_test fixture gmock absl::str_format) +target_link_libraries( + user_mesh_test + fixture + gmock + absl::str_format +) mujoco_test(user_composite_test) target_link_libraries(user_composite_test fixture gmock) diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc index 5d962045..4c6c1822 100644 --- a/test/xml/xml_native_writer_test.cc +++ b/test/xml/xml_native_writer_test.cc @@ -917,8 +917,8 @@ TEST_F(XMLWriterLocaleTest, IgnoresLocale) { mj_deleteModel(model); // Test that MuJoCo doesn't override locales for subsequent calls. - char formatted[7]; - std::snprintf(formatted, sizeof(formatted), "%f", 3.9375); + char formatted[8]; + std::snprintf(formatted, sizeof(formatted), "%.4f", 3.9375); EXPECT_EQ(std::string(formatted), "3,9375"); }