Skip to content

Commit

Permalink
Prevent allocation error (#73)
Browse files Browse the repository at this point in the history
* prevent allocation error and log stack trace

* improve fix for the allocation error

* Log stack traces when an allocation error is encountered
* Send a Supportability metric about the module length problem
* Fix linker error during axiom lib check in agent build

* increase agent version number

* fix compilation for zts builds

* fix drupal test compilation

* Remove -ldl in agent autoconf axiom check on BSD (#82)

* Remove -ldl in agent autoconf axiom check on BSD

* Move AXIOM_CHECK variable setting to system-dependent section

Co-authored-by: Kyle Kneitinger <kyle@kneit.in>
  • Loading branch information
Joshua Benuck and kneitinger committed Jan 22, 2021
1 parent b676ffb commit 3a850aa
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 14 deletions.
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9.15.0
9.16.0
14 changes: 13 additions & 1 deletion agent/config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ if test "$PHP_NEWRELIC" = "yes"; then
nrpic="-pic"
LDFLAGS="$LDFLAGS -pthread -lm"
EXTRA_LDFLAGS="$EXTRA_LDFLAGS -static-libgcc -export-symbols export.syms"
AXIOM_CHECK_LIB="axiom"
;;

esac
Expand Down Expand Up @@ -115,7 +116,18 @@ if test "$PHP_NEWRELIC" = "yes"; then
])

dnl Check for axiom.
PHP_CHECK_LIBRARY(axiom, nro_new, [
dnl The -ldl in the first argument is there to get the library check to pass.
dnl Without it, we get an undefined reference to dladdr as the dl lib must
dnl come after the axiom lib when linking. Note: this does not apply to
dnl FreeBSD builds, and if present, causes errors when building the agent.
dnl Earlier in this file, you will find a `case $host in` block for system
dnl dependent settings. If in the future, other systems require a modified
dnl axiom library check string, set them in that block, just as the
dnl `*freebsd*` pattern does.
if test -z "${AXIOM_CHECK_LIB}"; then
AXIOM_CHECK_LIB="axiom -ldl"
fi
PHP_CHECK_LIBRARY($AXIOM_CHECK_LIB, nro_new, [
PHP_ADD_INCLUDE($PHP_AXIOM)
dnl Avoid using PHP_ADD_LIBRARY and friends. They add an RPATH for
dnl the axiom directory, and there's no way to prevent it.
Expand Down
2 changes: 1 addition & 1 deletion agent/fw_drupal.c
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ static void nr_drupal_wrap_hook_within_module_invoke_all(

rv = module_invoke_all_parse_module_and_hook(
&module, &module_len, NRPRG(drupal_module_invoke_all_hook),
NRPRG(drupal_module_invoke_all_hook_len), func);
NRPRG(drupal_module_invoke_all_hook_len), func TSRMLS_CC);

if (NR_SUCCESS != rv) {
return;
Expand Down
27 changes: 24 additions & 3 deletions agent/fw_drupal_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "fw_hooks.h"
#include "fw_support.h"
#include "util_memory.h"
#include "util_signals.h"
#include "util_strings.h"

int nr_drupal_do_view_execute(const char* name,
Expand Down Expand Up @@ -127,7 +128,7 @@ nr_status_t module_invoke_all_parse_module_and_hook_from_strings(
const char* hook,
size_t hook_len,
const char* module_hook,
size_t module_hook_len) {
size_t module_hook_len TSRMLS_DC) {
size_t module_len = 0;
char* module = NULL;

Expand Down Expand Up @@ -172,6 +173,26 @@ nr_status_t module_invoke_all_parse_module_and_hook_from_strings(
- 1; /* Subtract 1 for underscore separator */
}

if (-1 >= (int)module_len) {
nrl_verbosedebug(NRL_FRAMEWORK,
"%s: module len is %d; ; "
"hook='%.*s'; module_hook='%.*s'",
__func__, (int)module_len, NRSAFELEN(hook_len), NRSAFESTR(hook),
NRSAFELEN(module_hook_len), NRSAFESTR(module_hook));

char* metric_name = NULL;
if (NR_FW_DRUPAL == NRPRG(current_framework)) {
metric_name = "Supportability/framework/Drupal/NegativeModuleLength";
} else if (NR_FW_DRUPAL8 == NRPRG(current_framework)) {
metric_name = "Supportability/framework/Drupal8/NegativeModuleLength";
}

if (metric_name != NULL) {
nrm_force_add(NRTXN(unscoped_metrics), metric_name, 0);
}
return NR_FAILURE;
}

module = nr_strndup(module_hook, module_len);

*module_ptr = module;
Expand All @@ -184,7 +205,7 @@ nr_status_t module_invoke_all_parse_module_and_hook(char** module_ptr,
size_t* module_len_ptr,
const char* hook,
size_t hook_len,
const zend_function* func) {
const zend_function* func TSRMLS_DC) {
const char* module_hook = NULL;
size_t module_hook_len = 0;

Expand All @@ -200,7 +221,7 @@ nr_status_t module_invoke_all_parse_module_and_hook(char** module_ptr,
module_hook_len = (size_t)nr_php_function_name_length(func);

return module_invoke_all_parse_module_and_hook_from_strings(
module_ptr, module_len_ptr, hook, hook_len, module_hook, module_hook_len);
module_ptr, module_len_ptr, hook, hook_len, module_hook, module_hook_len TSRMLS_CC);
}

void nr_drupal_headers_add(zval* arg, bool is_drupal_7 TSRMLS_DC) {
Expand Down
6 changes: 4 additions & 2 deletions agent/fw_drupal_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ nr_status_t module_invoke_all_parse_module_and_hook(char** module_ptr,
size_t* module_len_ptr,
const char* hook,
size_t hook_len,
const zend_function* func);
const zend_function* func
TSRMLS_DC);
/*
* Purpose : Given a function that is a hook function in a module, determine
* which component is the module and which is the hook, given that we
Expand All @@ -131,7 +132,8 @@ nr_status_t module_invoke_all_parse_module_and_hook_from_strings(
const char* hook,
size_t hook_len,
const char* module_hook,
size_t module_hook_len);
size_t module_hook_len
TSRMLS_DC);

/*
* Purpose: This function adds NR request headers for Drupal. arg is the second
Expand Down
11 changes: 6 additions & 5 deletions agent/tests/test_fw_drupal.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,21 @@ tlib_parallel_info_t parallel_info
static void test_single_extract_module_name_from_hook_and_hook_function(
const char* hook_function_name,
char* hook_name,
char* expected_module_name) {
char* expected_module_name
TSRMLS_DC) {
char* module = 0;
size_t module_len = 0;

module_invoke_all_parse_module_and_hook_from_strings(
&module, &module_len, hook_name, strlen(hook_name), hook_function_name,
strlen(hook_function_name));
strlen(hook_function_name) TSRMLS_CC);

tlib_pass_if_str_equal("Extracted Correct Module Name", module,
expected_module_name);
nr_free(module);
}

static void test_module_name(void) {
static void test_module_name(TSRMLS_D) {
int i = 0;
int number_of_fixtures = 0;

Expand All @@ -56,7 +57,7 @@ static void test_module_name(void) {

for (i = 0; i < number_of_fixtures; i++) {
test_single_extract_module_name_from_hook_and_hook_function(
fixtures[i][0], fixtures[i][1], fixtures[i][2]);
fixtures[i][0], fixtures[i][1], fixtures[i][2] TSRMLS_CC);
}
}

Expand Down Expand Up @@ -374,7 +375,7 @@ void test_main(void* p NRUNUSED) {
void*** tsrm_ls = NULL;
#endif /* ZTS && !PHP7 */
tlib_php_engine_create("" PTSRMLS_CC);
test_module_name();
test_module_name(TSRMLS_C);
test_drupal_headers_add(TSRMLS_C);
test_drupal_http_request_drupal_7(TSRMLS_C);
test_drupal_http_request_drupal_6(TSRMLS_C);
Expand Down
3 changes: 2 additions & 1 deletion axiom/nr_version.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@
*
* ube (9.14)
* vanilla (9.15)
* watermelon (9.16)
*/
#define NR_CODENAME "vanilla"
#define NR_CODENAME "watermelon"

const char* nr_version(void) {
return NR_STR2(NR_VERSION);
Expand Down
7 changes: 7 additions & 0 deletions axiom/util_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "util_logging.h"
#include "util_memory.h"
#include "util_signals.h"

#ifndef HAVE_REALLOCARRAY
#include <sys/types.h>
Expand All @@ -40,6 +41,7 @@ void* NRMALLOC NRMALLOCSZ(1) nr_malloc(size_t size) {
ret = (malloc)(size);
if (nrunlikely(0 == ret)) {
nrl_error(NRL_MEMORY, "failed to allocate %zu byte(s)", size);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand All @@ -56,6 +58,7 @@ void* NRMALLOC NRMALLOCSZ(1) nr_zalloc(size_t size) {
ret = (calloc)(1, size);
if (nrunlikely(0 == ret)) {
nrl_error(NRL_MEMORY, "failed to allocate %zu byte(s)", size);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand All @@ -76,6 +79,7 @@ void* NRMALLOC NRCALLOCSZ(1, 2) nr_calloc(size_t nelem, size_t elsize) {
ret = (calloc)(nelem, elsize);
if (nrunlikely(0 == ret)) {
nrl_error(NRL_MEMORY, "failed to allocate %zu x %zu bytes", nelem, elsize);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand All @@ -97,6 +101,7 @@ void* NRMALLOCSZ(2) nr_realloc(void* oldptr, size_t newsize) {
if (nrunlikely(0 == ret)) {
nrl_error(NRL_MEMORY, "failed to reallocate %p for %zu bytes", oldptr,
newsize);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand Down Expand Up @@ -166,6 +171,7 @@ char* NRMALLOC nr_strdup(const char* orig) {

if (NULL == ret) {
nrl_error(NRL_MEMORY | NRL_STRING, "failed to duplicate string %p", orig);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand Down Expand Up @@ -210,6 +216,7 @@ char* NRMALLOC nr_strndup(const char* orig, size_t len) {
if (nrunlikely(0 == ret)) {
nrl_error(NRL_MEMORY | NRL_STRING, "failed to duplicate string %p %zu",
orig, len);
nr_signal_tracer_common(31); /* SIGSYS(31) - bad system call */
exit(3);
}

Expand Down

0 comments on commit 3a850aa

Please sign in to comment.