APL-CORE: June 2020 Release of APL 1.3 compilant core engine (1.3.3)

For more details on this release refer to CHANGELOG.md

To learn about APL see:  https://developer.amazon.com/docs/alexa-presentation-language/understand-apl.html

Author: bddufour

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## [1.3.3]
+
+This release is a bug fix release of apl-core-library
+
+### Added
+
+- Added support for memory checks with valgrind during unit testing
+
+### Changed
+
+- Fixed windows build
+- Fixed a bug in Math.random implementation that caused random numbers not to use the full range of possible values
+
+
 ## [1.3.2]
 
 This release fixes a performance regression found in release 1.3.1.

README.md

@@ -165,9 +165,19 @@ To build `libaplcore.a` and tests in your favorite C++ environment, do the
 following:
 
 ```
- $ source apl-dev-env.sh
- $ apl-build-core
- $ apl-test-core
+ source apl-dev-env.sh
+
+ # Build the library
+ apl-build-core
+
+ # Run unit tests
+ apl-test-core
+
+ # Generate code coverage
+ apl-coverage-core
+
+ # Run memcheck
+ apl-check-core
 ```
 
 # Building APL Core + Tests (Windows)

apl-dev-env.sh

@@ -126,6 +126,15 @@ function apl-test-core {  # Run unit tests in the core build
     )
 }
 
+function apl-memcheck-core {  # Run unit tests in the core build
+    (
+        apl-switch-to-build-directory build $@ && \
+        $CMAKE -DBUILD_TESTS=ON  -DCOVERAGE=OFF .. && \
+        make -j$APL_BUILD_PROCS && \
+        valgrind --tool=memcheck --gen-suppressions=all --track-origins=yes --leak-check=full --num-callers=50 ./unit/unittest
+    )
+}
+
 function apl-coverage-core {  # Generate and print coverage report
     (
         apl-switch-to-build-directory build $@ && \

aplcore/src/datasource/dynamicindexlistdatasourceprovider.cpp

@@ -226,6 +226,13 @@ DynamicIndexListDataSourceConnection::processUpdate(DynamicIndexListUpdateType t
             << " is dead while trying to process update.";
         return false;
     }
+
+    if (index < mMinimumInclusiveIndex) {
+        provider->constructAndReportError(ERROR_REASON_LIST_INDEX_OUT_OF_RANGE,
+                                            shared_from_this(), index,
+                                            "Requested index out of bounds.");
+        return false;
+    }
     size_t idx = index - mMinimumInclusiveIndex;
 
     auto context = mContext.lock();
@@ -758,4 +765,4 @@ DynamicIndexListDataSourceProvider::constructAndReportError(
         const Object& operationIndex,
         const std::string& message) {
     constructAndReportError(reason, connection->getListId(), connection->getListVersion(), operationIndex, message);
-}
+}

aplcore/src/primitives/functions.cpp

@@ -14,17 +14,17 @@
  */
 
 #include <algorithm>
-#include <climits>
 #include <cmath>
 #ifdef APL_CORE_UWP
 #include <random>
 #endif
+#include <stdint.h>
 #include <stdlib.h>
 
+#include "apl/engine/context.h"
 #include "apl/primitives/functions.h"
 #include "apl/primitives/object.h"
 #include "apl/primitives/timefunctions.h"
-#include "apl/engine/context.h"
 #include "apl/primitives/timegrammar.h"
 
 namespace apl {
@@ -77,16 +77,20 @@ mathClamp(const std::vector<Object>& args)
 #ifdef APL_CORE_UWP
 static std::random_device random_device;
 static std::mt19937 generator(random_device());
-static unsigned long random() { return generator(); }
 #endif
 
 static Object
 mathRandom(const std::vector<Object>& args)
 {
 #ifdef HAVE_DECL_ARC4RANDOM
-    return arc4random() / ((double) ULONG_MAX);
+    // By contract, arc4random can return any valid uint32_t value
+    return arc4random() / ((double) UINT32_MAX);
+#elif APL_CORE_UWP
+    // By contract, std::mt19937 returns values between 0 and mt19937::max()
+    return generator() / ((double) std::mt19937::max());
 #else
-    return ((unsigned long)random()) / ((double) ULONG_MAX);
+    // By contract, random() returns values between 0 and RAND_MAX
+    return random() / ((double) RAND_MAX);
 #endif
 }
 

aplcore/src/primitives/radii.cpp

@@ -13,6 +13,8 @@
  * permissions and limitations under the License.
  */
 
+#include <string>
+
 #include "apl/primitives/radii.h"
 #include "apl/utils/streamer.h"
 

components.cmake

@@ -80,11 +80,16 @@ endif(BUILD_DOC)
 
 # Test cases are built conditionally. Only affect core do not build them for everything else.
 if (BUILD_TESTS)
-    enable_testing()
+    include(CTest)
     include_directories(${GTEST_INCLUDE})
     add_subdirectory(unit)
     add_subdirectory(test)
     if (TELEMETRY)
         add_subdirectory(performance)
     endif(TELEMETRY)
+    set(MEMCHECK_OPTIONS "--tool=memcheck --leak-check=full --show-reachable=no --error-exitcode=1 --errors-for-leak-kinds=definite,possible")
+    add_custom_target(unittest_memcheck
+            COMMAND ${CMAKE_CTEST_COMMAND} -VV
+            --overwrite MemoryCheckCommandOptions=${MEMCHECK_OPTIONS}
+            -T memcheck)
 endif (BUILD_TESTS)

gcc.cmake

@@ -25,18 +25,18 @@ if(COVERAGE)
 
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} --coverage -fprofile-arcs -ftest-coverage")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage -fprofile-arcs -ftest-coverage")
-    set(COVERAGE_CLEANED "${coverage_info}.cleaned")
 
     function(target_add_code_coverage EXEC_NAME TARGET_NAME)
         set(COVERAGE_INFO "${CMAKE_BINARY_DIR}/${TARGET_NAME}.info")
+        set(COVERAGE_CLEANED "${COVERAGE_INFO}.cleaned")
 
         add_custom_target(coverage-${TARGET_NAME}
                 ${LCOV_PATH} --directory . --zerocounters
                 COMMAND $<TARGET_FILE:${EXEC_NAME}>
-                COMMAND ${LCOV_PATH} --directory ${CMAKE_SOURCE_DIR} --capture --output-file ${COVERAGE_INFO}
-                COMMAND ${LCOV_PATH} --remove ${COVERAGE_INFO} '**/unit/*' '/usr/*' '**/build/*' '**/thirdparty/*' --output-file ${COVERAGE_CLEANED}
+                COMMAND ${LCOV_PATH} --directory ${CMAKE_BINARY_DIR} --capture --output-file ${COVERAGE_INFO}
+                COMMAND ${LCOV_PATH} --remove ${COVERAGE_INFO} '**/unit/*' '/usr/*' '**/*build/*' '**/thirdparty/*' --output-file ${COVERAGE_CLEANED}
                 COMMAND ${GENHTML_PATH} -o ${CMAKE_BINARY_DIR}/coverage ${COVERAGE_CLEANED}
-                COMMAND ${CMAKE_COMMAND} -E remove ${COVERAGE_INFO} ${COVERAGE_CLEANED}
+                COMMAND ${CMAKE_COMMAND} -E remove ${COVERAGE_INFO}
                 DEPENDS ${TARGET_NAME} ${EXEC_NAME}
                 )
         add_custom_command(TARGET coverage-${TARGET_NAME} POST_BUILD

thirdparty/thirdparty.cmake

@@ -25,10 +25,6 @@ list(APPEND CMAKE_ARGS -DCMAKE_CXX_FLAGS=${EXT_CXX_ARGS})
 list(APPEND CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/lib)
 list(APPEND CMAKE_ARGS -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE})
 
-if("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC")
-    set(PATCH_FLAGS "--binary")
-endif()
-
 ExternalProject_Add(yoga
         URL ${APL_PROJECT_DIR}/thirdparty/yoga-1.16.0.tar.gz
         URL_MD5 c9e88076ec371513fb23a0a5370ec2fd

unit/CMakeLists.txt

@@ -107,9 +107,19 @@ add_executable( unittest
 add_subdirectory(${CMAKE_BINARY_DIR}/googletest-src
         ${CMAKE_BINARY_DIR}/googletest-build
         EXCLUDE_FROM_ALL)
-
-target_link_libraries(unittest apl gtest gtest_main)
-
 if(COVERAGE)
     target_add_code_coverage(unittest apl)
 endif()
+target_link_libraries(unittest apl gtest gtest_main)
+if(CTEST_INDIVIDUALLY)
+    # NOTE: discovered ctest targets below are much slower than their direct counterparts.  Ctest loads
+    #       tests individually instead of just running all in a class.  This makes it take much
+    #       longer for execution.  This is somewhat useful if you want to execute tests using ctest scripts, but is
+    #       completely unusable on dev machine: each test takes 800ms vs 20ms, and valgrind takes 4-5s per test.
+    include(GoogleTest)
+    gtest_discover_tests(unittest)
+else()
+    # Adds the entire unittest executable as a single ctest.  Great for speed.
+    add_test(all-tests unittest)
+endif()
+