Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't read protobuf static data across shared library lines directly #6979

Merged
merged 23 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,6 @@ coverage_report/

# ctest
/Testing/

# protobuf
!wpiprotoplugin.jar
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ if(WITH_JAVA OR WITH_JAVA_SOURCE)
set(CMAKE_JAVA_COMPILE_FLAGS "-encoding" "UTF8" "-Xlint:unchecked")
find_package(Java REQUIRED COMPONENTS Development)
find_package(JNI REQUIRED COMPONENTS JVM)
else()
# Protoc requires the java runtime
find_package(Java REQUIRED COMPONENTS Runtime)
endif()

find_package(LIBSSH 0.7.1)
Expand Down Expand Up @@ -273,6 +276,8 @@ if(WITH_NTCORE)
add_subdirectory(ntcore)
endif()

add_subdirectory(protoplugin)

if(WITH_WPIMATH)
if(WITH_JAVA)
set(WPIUNITS_DEP_REPLACE ${WPIUNITS_DEP_REPLACE_IMPL})
Expand Down
2 changes: 1 addition & 1 deletion buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ repositories {
}
}
dependencies {
implementation "edu.wpi.first:native-utils:2025.1.0"
implementation "edu.wpi.first:native-utils:2025.2.0"
}
20 changes: 20 additions & 0 deletions protoplugin/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
project(protoplugin)

set(PROTO_JAR ${CMAKE_CURRENT_SOURCE_DIR}/binary/wpiprotoplugin.jar)

configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/binary/wpiprotoplugin.sh.in
${CMAKE_CURRENT_BINARY_DIR}/wpiprotoplugin.sh
@ONLY
)
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/binary/wpiprotoplugin.bat.in
${CMAKE_CURRENT_BINARY_DIR}/wpiprotoplugin.bat
@ONLY
)

if(WIN32)
set(PROTOC_WPILIB_PLUGIN ${CMAKE_CURRENT_BINARY_DIR}/wpiprotoplugin.bat PARENT_SCOPE)
else()
set(PROTOC_WPILIB_PLUGIN ${CMAKE_CURRENT_BINARY_DIR}/wpiprotoplugin.sh PARENT_SCOPE)
endif()
2 changes: 2 additions & 0 deletions protoplugin/binary/wpiprotoplugin.bat.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@ECHO OFF
"@Java_JAVA_EXECUTABLE@" -jar "@PROTO_JAR@" %*
Binary file added protoplugin/binary/wpiprotoplugin.jar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have a CI check similar to pregenerate.yml or upstream-utils.yml that makes sure the jar file matches the jar file generated from protoplugin.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably add that in a later PR. Right now we need to work on getting this in because we're very broken right now.

Binary file not shown.
2 changes: 2 additions & 0 deletions protoplugin/binary/wpiprotoplugin.sh.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/sh
exec "@Java_JAVA_EXECUTABLE@" -jar "@PROTO_JAR@" "$@"
33 changes: 33 additions & 0 deletions protoplugin/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
plugins {
id 'application'
id 'java'
id 'com.gradleup.shadow' version '8.3.0'
}

application {
mainClass = "org.wpilib.ProtoCDllGenerator"
}

repositories {
mavenCentral()
}

dependencies {
implementation "com.google.protobuf:protobuf-java:3.21.12"
}

java {
sourceCompatibility = 8
targetCompatibility = 8
}

def cshadow = project.tasks.register("copyShadow", Copy) {
from(tasks.shadowJar.archiveFile) {
rename {
"wpiprotoplugin.jar"
}
}
into layout.projectDirectory.dir("binary")
}

build.dependsOn cshadow
1 change: 1 addition & 0 deletions protoplugin/settings.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
76 changes: 76 additions & 0 deletions protoplugin/src/main/java/org/wpilib/ProtoCDllGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) FIRST and other WPILib contributors.
// Open Source Software; you can modify and/or share it under the terms of
// the WPILib BSD license file in the root directory of this project.

package org.wpilib;

import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

import com.google.protobuf.DescriptorProtos.FileDescriptorProto;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Descriptors.Descriptor;
import com.google.protobuf.Descriptors.DescriptorValidationException;
import com.google.protobuf.Descriptors.FieldDescriptor;
import com.google.protobuf.Descriptors.FieldDescriptor.Type;
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest;

public final class ProtoCDllGenerator {
private ProtoCDllGenerator() {
}

public static void main(String[] args) throws IOException, DescriptorValidationException {
CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in);

Map<String, Descriptors.FileDescriptor> descriptors = new HashMap<>();

Descriptors.FileDescriptor[] tmpArray = new Descriptors.FileDescriptor[0];

for (FileDescriptorProto proto : request.getProtoFileList()) {
// Make array of file descriptors that exists
Descriptors.FileDescriptor[] depArray = descriptors.values().toArray(tmpArray);
Descriptors.FileDescriptor desc = Descriptors.FileDescriptor.buildFrom(proto, depArray);
descriptors.put(proto.getName(), desc);
}

// Filter to generated descriptors
CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder();

for (String genFile : request.getFileToGenerateList()) {
Descriptors.FileDescriptor desc = descriptors.get(genFile);

for (Descriptor msg : desc.getMessageTypes()) {
for (FieldDescriptor field : msg.getFields()) {
if (field.getType() == Type.MESSAGE && !field.isRepeated()) {
// If we have a nested non repeated field, we need a custom accessor
String type = field.getMessageType().getFullName();
type = "::" + type.replaceAll("\\.", "::");

// Add definition
response.addFileBuilder()
.setName(desc.getName().replace("proto", "pb.h"))
.setInsertionPoint("class_scope:" + msg.getFullName())
.setContent("// Custom WPILib Accessor\n"
+ "bool wpi_has_" + field.getName() + "() const;\n"
+ "const " + type + "& wpi_" + field.getName() + "() const;\n");

// Add implementation. As were in the cc file for the proto
// we can just call straight through to the inline definitions
response.addFileBuilder()
.setName(desc.getName().replace("proto", "pb.cc"))
.setInsertionPoint("namespace_scope")
.setContent("// Custom WPILib Accessor\n"
+ "bool " + msg.getName() + "::wpi_has_" + field.getName() + "() const { return has_"
+ field.getName() + "(); }\n"
+ "const " + type + "& " + msg.getName() + "::wpi_" + field.getName() + "() const { return "
+ field.getName() + "(); }\n");
}
}
}
}
response.build().writeTo(System.out);
}
}
11 changes: 11 additions & 0 deletions shared/java/javacommon.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,19 @@ protobuf {
protoc {
artifact = 'com.google.protobuf:protoc:3.21.12'
}
plugins {
wpilib {
path = rootProject.file("protoplugin/binary/wpiprotoplugin.jar")
}
}
generateProtoTasks {
all().configureEach { task ->
task.inputs.file(rootProject.file("protoplugin/binary/wpiprotoplugin.jar"))
task.plugins {
wpilib {
outputSubDir = 'cpp'
}
}
task.builtins {
cpp {}
remove java
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 14:13:07 -0700
Subject: [PATCH 01/12] Fix sign-compare warnings
Subject: [PATCH 01/13] Fix sign-compare warnings

---
src/google/protobuf/compiler/importer.cc | 2 +-
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 14:41:39 -0700
Subject: [PATCH 02/12] Remove redundant move
Subject: [PATCH 02/13] Remove redundant move

---
src/google/protobuf/extension_set.h | 2 +-
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 15:00:20 -0700
Subject: [PATCH 03/12] Fix maybe-uninitialized warnings
Subject: [PATCH 03/13] Fix maybe-uninitialized warnings

---
src/google/protobuf/arena.cc | 6 +++---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 15:03:38 -0700
Subject: [PATCH 04/12] Fix coded_stream WriteRaw
Subject: [PATCH 04/13] Fix coded_stream WriteRaw

---
src/google/protobuf/implicit_weak_message.h | 2 +-
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 15:13:45 -0700
Subject: [PATCH 05/12] Suppress enum-enum conversion warning
Subject: [PATCH 05/13] Suppress enum-enum conversion warning

---
src/google/protobuf/generated_message_tctable_impl.h | 9 +++++++++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 15:16:46 -0700
Subject: [PATCH 06/12] Fix noreturn function returning
Subject: [PATCH 06/13] Fix noreturn function returning

---
src/google/protobuf/generated_message_tctable_impl.h | 3 +++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Sat, 10 Jun 2023 15:59:45 -0700
Subject: [PATCH 07/12] Work around GCC 12 restrict warning compiler bug
Subject: [PATCH 07/13] Work around GCC 12 restrict warning compiler bug

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105329
---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Tue, 13 Jun 2023 23:56:15 -0700
Subject: [PATCH 08/12] Disable MSVC switch warning
Subject: [PATCH 08/13] Disable MSVC switch warning

---
src/google/protobuf/generated_message_reflection.cc | 4 ++++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Tue, 13 Jun 2023 23:58:50 -0700
Subject: [PATCH 09/12] Disable unused function warning
Subject: [PATCH 09/13] Disable unused function warning

---
src/google/protobuf/generated_message_tctable_lite.cc | 4 ++++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Wed, 14 Jun 2023 00:02:26 -0700
Subject: [PATCH 10/12] Disable pedantic warning
Subject: [PATCH 10/13] Disable pedantic warning

---
src/google/protobuf/descriptor.h | 8 ++++++++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Johnson <[email protected]>
Date: Mon, 9 Oct 2023 19:28:08 -0700
Subject: [PATCH 11/12] Avoid use of sprintf
Subject: [PATCH 11/13] Avoid use of sprintf

---
src/google/protobuf/stubs/strutil.cc | 14 +++++++++++---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tyler Veness <[email protected]>
Date: Fri, 10 Nov 2023 14:17:53 -0800
Subject: [PATCH 12/12] Suppress stringop-overflow warning false positives
Subject: [PATCH 12/13] Suppress stringop-overflow warning false positives

---
src/google/protobuf/io/coded_stream.h | 7 +++++++
Expand Down
Loading
Loading