Skip to content

Commit

Permalink
chore: native asm code linting (#9316)
Browse files Browse the repository at this point in the history
## Description

This includes many small linting-style changes including:

- const correctness
- include header cleanup (which actually improves compile time).
- C++ conventions (no underscores, C++ casts).
- et cetera...

## Checklist

- [X] Change(s) are motivated and described in the PR description
- [X] Testing strategy is described if automated tests are not included
in the PR
- [X] Risks are described (performance impact, potential for breakage,
maintainability)
- [X] Change is maintainable (easy to change, telemetry, documentation)
- [X] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [X] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [X] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [X] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [ ] Title is accurate
- [ ] All changes are related to the pull request's stated goal
- [ ] Description motivates each change
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [ ] Testing strategy adequately addresses listed risks
- [ ] Change is maintainable (easy to change, telemetry, documentation)
- [ ] Release note makes sense to a user of the library
- [ ] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [ ] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Signed-off-by: Juanjo Alvarez <[email protected]>
  • Loading branch information
juanjux authored May 20, 2024
1 parent 9d6ad17 commit a6735c5
Show file tree
Hide file tree
Showing 31 changed files with 234 additions and 307 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @return PyObject*: return None (Remember, Pyobject None isn't the same as nullptr)
*/
PyObject*
api_extend_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
api_extend_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs)
{
if (nargs != 2 or !args) {
throw py::value_error(MSG_ERROR_N_PARAMS);
Expand Down
8 changes: 4 additions & 4 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectFormat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ api_format_aspect(StrType& candidate_text,

if (!ranges_orig.empty() or !candidate_text_ranges.empty()) {
auto new_template =
_int_as_formatted_evidence<StrType>(candidate_text, candidate_text_ranges, TagMappingMode::Mapper);
int_as_formatted_evidence<StrType>(candidate_text, candidate_text_ranges, TagMappingMode::Mapper);

py::list new_args;
py::dict new_kwargs;
for (const auto arg : args) {
if (is_text(arg.ptr())) {
auto str_arg = py::cast<py::str>(arg);
auto n_arg = _all_as_formatted_evidence<py::str>(str_arg, TagMappingMode::Mapper);
auto n_arg = all_as_formatted_evidence<py::str>(str_arg, TagMappingMode::Mapper);
new_args.append(n_arg);
} else {
new_args.append(arg);
Expand All @@ -43,15 +43,15 @@ api_format_aspect(StrType& candidate_text,
for (auto [key, value] : kwargs) {
if (is_text(value.ptr())) {
auto str_value = py::cast<py::str>(value);
auto n_value = _all_as_formatted_evidence<py::str>(str_value, TagMappingMode::Mapper);
auto n_value = all_as_formatted_evidence<py::str>(str_value, TagMappingMode::Mapper);
new_kwargs[key] = n_value;
} else {
new_kwargs[key] = value;
}
}
StrType new_template_format =
py::getattr(new_template, "format")(*(py::cast<py::tuple>(new_args)), **new_kwargs);
std::tuple result = _convert_escaped_text_to_taint_text<StrType>(new_template_format, ranges_orig);
std::tuple result = convert_escaped_text_to_taint_text<StrType>(new_template_format, ranges_orig);
StrType result_text = get<0>(result);
TaintRangeRefs result_ranges = get<1>(result);
PyObject* new_result = new_pyobject_id(result_text.ptr());
Expand Down
3 changes: 0 additions & 3 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectFormat.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
#pragma once
#include "Aspects/Helpers.h"
#include "Initializer/Initializer.h"
#include "TaintTracking/Source.h"
#include "TaintTracking/TaintRange.h"
#include "TaintTracking/TaintedObject.h"

template<class StrType>
StrType
Expand Down
13 changes: 5 additions & 8 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
PyObject*
index_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* idx, const TaintRangeMapTypePtr& tx_taint_map)
{
auto idx_long = PyLong_AsLong(idx);
bool ranges_error;
TaintRangeRefs ranges_to_set, ranges;
std::tie(ranges, ranges_error) = get_ranges(candidate_text, tx_taint_map);
const auto idx_long = PyLong_AsLong(idx);
TaintRangeRefs ranges_to_set;
auto [ranges, ranges_error] = get_ranges(candidate_text, tx_taint_map);
if (ranges_error) {
return result_o;
}
Expand All @@ -38,7 +37,7 @@ index_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* idx, const
}

PyObject*
api_index_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
api_index_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs)
{
if (nargs != 2) {
py::set_error(PyExc_ValueError, MSG_ERROR_N_PARAMS);
Expand All @@ -49,9 +48,7 @@ api_index_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
PyObject* candidate_text = args[0];
PyObject* idx = args[1];

PyObject* result_o;

result_o = PyObject_GetItem(candidate_text, idx);
PyObject* result_o = PyObject_GetItem(candidate_text, idx);

if (not ctx_map or ctx_map->empty()) {
return result_o;
Expand Down
1 change: 0 additions & 1 deletion ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectIndex.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#pragma once
#include "Aspects/Helpers.h"
#include "TaintedOps/TaintedOps.h"

PyObject*
Expand Down
7 changes: 3 additions & 4 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ aspect_join(PyObject* sep, PyObject* result, PyObject* iterable_elements, const
// b"a".join(u"c", b"d") -> unicode
const size_t& element_len = get_pyobject_size(element);
if (element_len > 0) {
const auto& to_element = get_tainted_object(element, tx_taint_map);
if (to_element) {
if (const auto& to_element = get_tainted_object(element, tx_taint_map)) {
if (current_pos == 0 and !first_tainted_to) {
first_tainted_to = to_element;
} else {
Expand Down Expand Up @@ -138,7 +137,7 @@ aspect_join(PyObject* sep, PyObject* result, PyObject* iterable_elements, const
}

PyObject*
api_join_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
api_join_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs)
{
if (nargs != 2) {
py::set_error(PyExc_ValueError, MSG_ERROR_N_PARAMS);
Expand All @@ -152,7 +151,7 @@ api_join_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
if (PyIter_Check(arg0) or PySet_Check(arg0) or PyFrozenSet_Check(arg0)) {
PyObject* iterator = PyObject_GetIter(arg0);

if (iterator != NULL) {
if (iterator != nullptr) {
PyObject* item;
PyObject* list_aux = PyList_New(0);
while ((item = PyIter_Next(iterator))) {
Expand Down
2 changes: 0 additions & 2 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#pragma once
#include "Initializer/Initializer.h"
#include "TaintTracking/TaintRange.h"
#include "TaintTracking/TaintedObject.h"

namespace py = pybind11;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ add_aspect(PyObject* result_o,
PyObject* text_to_add,
const TaintRangeMapTypePtr& tx_taint_map)
{
size_t len_candidate_text{ get_pyobject_size(candidate_text) };
size_t len_text_to_add{ get_pyobject_size(text_to_add) };
const size_t len_candidate_text{ get_pyobject_size(candidate_text) };
const size_t len_text_to_add{ get_pyobject_size(text_to_add) };

if (len_text_to_add == 0 and len_candidate_text > 0) {
return candidate_text;
Expand Down Expand Up @@ -48,7 +48,7 @@ add_aspect(PyObject* result_o,
}

auto tainted = initializer->allocate_tainted_object_copy(to_candidate_text);
tainted->add_ranges_shifted(to_text_to_add, (RANGE_START)len_candidate_text);
tainted->add_ranges_shifted(to_text_to_add, static_cast<RANGE_START>(len_candidate_text));
const auto res_new_id = new_pyobject_id(result_o);
Py_DecRef(result_o);
set_tainted_object(res_new_id, tainted, tx_taint_map);
Expand Down Expand Up @@ -80,7 +80,7 @@ api_add_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
PyObject* candidate_text = args[0];
PyObject* text_to_add = args[1];

PyObject* result_o;
PyObject* result_o = nullptr;
if (PyUnicode_Check(candidate_text)) {
result_o = PyUnicode_Concat(candidate_text, text_to_add);
} else if (PyBytes_Check(candidate_text)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#pragma once
#include "Initializer/Initializer.h"
#include "TaintTracking/TaintRange.h"
#include "TaintTracking/TaintedObject.h"

PyObject*
api_add_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs);
40 changes: 20 additions & 20 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectSlice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@
* @return A map of taint ranges for the given index range map.
*/
TaintRangeRefs
reduce_ranges_from_index_range_map(TaintRangeRefs index_range_map)
reduce_ranges_from_index_range_map(const TaintRangeRefs& index_range_map)
{
TaintRangeRefs new_ranges;
TaintRangePtr current_range;
size_t current_start = 0;
size_t index;

for (index = 0; index < index_range_map.size(); ++index) {
auto taint_range{ index_range_map.at(index) };
if (taint_range != current_range) {
if (const auto& taint_range{ index_range_map.at(index) }; taint_range != current_range) {
if (current_range) {
new_ranges.emplace_back(
initializer->allocate_taint_range(current_start, index - current_start, current_range->source));
Expand All @@ -26,7 +25,7 @@ reduce_ranges_from_index_range_map(TaintRangeRefs index_range_map)
current_start = index;
}
}
if (current_range != NULL) {
if (current_range != nullptr) {
new_ranges.emplace_back(
initializer->allocate_taint_range(current_start, index - current_start, current_range->source));
}
Expand All @@ -38,6 +37,9 @@ reduce_ranges_from_index_range_map(TaintRangeRefs index_range_map)
*
* @param text The text object for which the taint ranges are to be built.
* @param ranges The taint range map that stores taint information.
* @param start The start index of the text object.
* @param stop The stop index of the text object.
* @param step The step index of the text object.
*
* @return A map of taint ranges for the given text object.
*/
Expand All @@ -57,7 +59,7 @@ build_index_range_map(PyObject* text, TaintRangeRefs& ranges, PyObject* start, P
index++;
}
}
long length_text = (long long)py::len(text);
long length_text = static_cast<long long>(py::len(text));
while (index < length_text) {
index_range_map.emplace_back(nullptr);
index++;
Expand All @@ -71,7 +73,7 @@ build_index_range_map(PyObject* text, TaintRangeRefs& ranges, PyObject* start, P
}
}
long stop_int = length_text;
if (stop != NULL) {
if (stop != nullptr) {
stop_int = PyLong_AsLong(stop);
if (stop_int > length_text) {
stop_int = length_text;
Expand All @@ -83,7 +85,7 @@ build_index_range_map(PyObject* text, TaintRangeRefs& ranges, PyObject* start, P
}
}
long step_int = 1;
if (step != NULL) {
if (step != nullptr) {
step_int = PyLong_AsLong(step);
}
for (auto i = start_int; i < stop_int; i += step_int) {
Expand All @@ -101,9 +103,7 @@ slice_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* start, PyOb
if (not ctx_map or ctx_map->empty()) {
return result_o;
}
bool ranges_error;
TaintRangeRefs ranges;
std::tie(ranges, ranges_error) = get_ranges(candidate_text, ctx_map);
auto [ranges, ranges_error] = get_ranges(candidate_text, ctx_map);
if (ranges_error or ranges.empty()) {
return result_o;
}
Expand All @@ -117,15 +117,15 @@ PyObject*
api_slice_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
{
if (nargs < 3) {
return NULL;
return nullptr;
}
PyObject* candidate_text = args[0];
PyObject* start = PyLong_FromLong(0);

if (PyNumber_Check(args[1])) {
start = PyNumber_Long(args[1]);
}
PyObject* stop = NULL;
PyObject* stop = nullptr;
if (PyNumber_Check(args[2])) {
stop = PyNumber_Long(args[2]);
}
Expand All @@ -137,30 +137,30 @@ api_slice_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
}

PyObject* slice = PySlice_New(start, stop, step);
if (slice == NULL) {
if (slice == nullptr) {
PyErr_Print();
if (start != NULL) {
if (start != nullptr) {
Py_DecRef(start);
}
if (stop != NULL) {
if (stop != nullptr) {
Py_DecRef(stop);
}
if (step != NULL) {
if (step != nullptr) {
Py_DecRef(step);
}
return NULL;
return nullptr;
}
PyObject* result = PyObject_GetItem(candidate_text, slice);

auto res = slice_aspect(result, candidate_text, start, stop, step);

if (start != NULL) {
if (start != nullptr) {
Py_DecRef(start);
}
if (stop != NULL) {
if (stop != nullptr) {
Py_DecRef(stop);
}
if (step != NULL) {
if (step != nullptr) {
Py_DecRef(step);
}
Py_DecRef(slice);
Expand Down
1 change: 0 additions & 1 deletion ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectSlice.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#pragma once
#include "Aspects/Helpers.h"
#include "TaintedOps/TaintedOps.h"

PyObject*
Expand Down
21 changes: 9 additions & 12 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectSplit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ template<class StrType>
py::list
api_split_text(const StrType& text, const optional<StrType>& separator, const optional<int> maxsplit)
{
auto split = text.attr("split");
auto split_result = split(separator, maxsplit);
const auto split = text.attr("split");
const auto split_result = split(separator, maxsplit);

const auto tx_map = initializer->get_tainting_map();
if (not tx_map or tx_map->empty()) {
return split_result;
}

auto ranges = api_get_ranges(text);
if (not ranges.empty()) {
if (auto ranges = api_get_ranges(text); not ranges.empty()) {
set_ranges_on_splitted(text, ranges, split_result, tx_map, false);
}

Expand All @@ -25,15 +24,14 @@ template<class StrType>
py::list
api_rsplit_text(const StrType& text, const optional<StrType>& separator, const optional<int> maxsplit)
{
auto rsplit = text.attr("rsplit");
auto split_result = rsplit(separator, maxsplit);
const auto rsplit = text.attr("rsplit");
const auto split_result = rsplit(separator, maxsplit);
const auto tx_map = initializer->get_tainting_map();
if (not tx_map or tx_map->empty()) {
return split_result;
}

auto ranges = api_get_ranges(text);
if (not ranges.empty()) {
if (auto ranges = api_get_ranges(text); not ranges.empty()) {
set_ranges_on_splitted(text, ranges, split_result, tx_map, false);
}
return split_result;
Expand All @@ -43,15 +41,14 @@ template<class StrType>
py::list
api_splitlines_text(const StrType& text, bool keepends)
{
auto splitlines = text.attr("splitlines");
auto split_result = splitlines(keepends);
const auto splitlines = text.attr("splitlines");
const auto split_result = splitlines(keepends);
const auto tx_map = initializer->get_tainting_map();
if (not tx_map or tx_map->empty()) {
return split_result;
}

auto ranges = api_get_ranges(text);
if (not ranges.empty()) {
if (auto ranges = api_get_ranges(text); not ranges.empty()) {
set_ranges_on_splitted(text, ranges, split_result, tx_map, keepends);
}
return split_result;
Expand Down
4 changes: 2 additions & 2 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

template<class StrType>
py::list
api_split_text(const StrType& text, const optional<StrType>& separator, const optional<int> maxsplit);
api_split_text(const StrType& text, const optional<StrType>& separator, optional<int> maxsplit);

template<class StrType>
py::list
api_rsplit_text(const StrType& text, const optional<StrType>& separator, const optional<int> maxsplit);
api_rsplit_text(const StrType& text, const optional<StrType>& separator, optional<int> maxsplit);

template<class StrType>
py::list
Expand Down
Loading

0 comments on commit a6735c5

Please sign in to comment.